Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[3/n] Remove u24 #521

Merged
merged 1 commit into from
Jun 29, 2021
Merged

[3/n] Remove u24 #521

merged 1 commit into from
Jun 29, 2021

Conversation

Dirbaio
Copy link
Contributor

@Dirbaio Dirbaio commented Jun 27, 2021

Part 3 of N of #492

  • Now that we're going to use rzCOBS for encoding the stream, extra zero bytes are not that expenseive. Using a u32 instead of a u24 adds one zero byte, which when encoded is just 1 extra bit.
  • Users are unlikely to use u24, as it's quite obscure (I didn't know it existed until I found it while reading defmt's source).
  • It makes the code more complicated, because it's not natively supported by Rust. In the code size optimizations of the macro codegen I'm working on, it really breaks the "symmetry" of the code.

Therefore I propose we remove it.

@Dirbaio Dirbaio mentioned this pull request Jun 27, 2021
Merged
@Urhengulas Urhengulas requested a review from japaric June 28, 2021 09:03
japaric
japaric approved these changes Jun 29, 2021
View reviewed changes
Copy link
Member

@japaric japaric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds reasonable to me!

@japaric
Copy link
Member

japaric commented Jun 29, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 29, 2021

Build succeeded:

@bors bors bot merged commit 0a8d65b into knurling-rs:main Jun 29, 2021
bors bot added a commit that referenced this pull request Jun 30, 2021
@bors @Dirbaio
Merge #508
5abfc33 
508: [5/n] Format trait v2 r=japaric a=Dirbaio

Part of #492 . Depends on ~~#505 #507 #521~~ #524

- Implemented new Format trait according to #492.
- Adjusted everything else accordingly.

Co-authored-by: Dario Nieuwenhuis <[email protected]>
Urhengulas added a commit that referenced this pull request Jul 7, 2021
@Urhengulas
book::primitives: Table-ize and drop u24 (got dropped in (#521))
4dbdc9f
Urhengulas added a commit that referenced this pull request Jul 13, 2021
@Urhengulas
book First refactor-through
3a1b78a 
* `setup`: Deduplicate setup
* `setup-app`: name `cargo.toml`; add `defmt-itm`
* `macros`: Language
* `primitives`: Table-ize and drop `u24` (got dropped in (#521))
* `str`: Add links to doc-rs
* ...
Urhengulas added a commit that referenced this pull request Jul 13, 2021
@Urhengulas
book First refactor-through
37d05b9 
* `setup`: Deduplicate setup
* `setup-app`: name `cargo.toml`; add `defmt-itm`
* `macros`: Language
* `primitives`: Table-ize and drop `u24` (got dropped in (#521))
* `str`: Add links to doc-rs
* ...
@Urhengulas Urhengulas mentioned this pull request Jul 21, 2021
Merged
7 tasks
bors bot added a commit that referenced this pull request Oct 19, 2021
@bors @Urhengulas
Merge #547
da1682f 
547: Migration guide `v0.2.x` to `v0.3.0` r=japaric a=Urhengulas

Migration guide from `defmt v0.2.x` to version `v0.3.0`.

https://deploy-preview-547--admiring-dubinsky-56dff5.netlify.app/migration-02-03.html

Fixes #530.

### TODO
- [x] #505: Logger trait v2
- [x] #521: [3/n] Remove u24
- [x] #522: Replace `µs` hint with `us`
- [x] (no adaption needed) ~~#508: [5/n] Format trait v2~~
- [x] #519: `DEFMT_LOG`
- [x] #550: `defmt::flush()`
- [x] knurling-rs/probe-run#198, knurling-rs/probe-run#250, 

Co-authored-by: Johann Hemmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@japaric japaric japaric approved these changes

Assignees

@japaric japaric

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Dirbaio @japaric