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

Implement i128 and u128. #284

Merged
merged 13 commits into from
Dec 3, 2020
Merged

Implement i128 and u128. #284

merged 13 commits into from
Dec 3, 2020

Conversation

BriocheBerlin
Copy link
Contributor

Closes #199

@jonas-schievink jonas-schievink added this to the v0.2.0 milestone Nov 27, 2020
japaric
japaric approved these changes Nov 27, 2020
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.

Looks great! Extending the format string is a breaking change so this needs to wait until v0.2.0 but I'll mark this as approved.

firmware/qemu/src/bin/log.rs
i128::min_value()
);

defmt::info!(
Copy link
Member

Choose a reason for hiding this comment

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

could you also test here that we can format u128 and i128 values using the {:?} format specifier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it ok now?

Copy link
Member

Choose a reason for hiding this comment

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

yes, looks good; thanks! I'll re-approve the PR but it will need a rebase before it can be merged.

@japaric japaric added the breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version label Nov 27, 2020
justahero and others added 11 commits December 1, 2020 18:05
@justahero @BriocheBerlin
Add tests to parse i64 / u64 parameters
c4382de
@justahero @BriocheBerlin
Add support to parse i128 parameter
6c53186
@justahero @BriocheBerlin
Add support to decode i128 argument
ab7f75f 
* expand test to include `i64` and `i128` types
@justahero @BriocheBerlin
Implement i128 function in Formatter
ac0cf0f 
Use the new i128 argument in qemu example `log`.

Please **note** that this currently leads to "defmt version mismatch"
when executing the log binary.
@BriocheBerlin
Adds support to parse u128.
4068e2c
@justahero @BriocheBerlin
Update snapshots to include i128 output
fa1b712 
The change updates the `log.rs` program to log i128 type.

Co-authored-by: Brigitte <brigitte.markmann@asquera.de>
@BriocheBerlin
Decode u128 argument (and missing parts for u64).
53276f8
@BriocheBerlin
u128 function in Formatter.
09e22c9
@BriocheBerlin
Add snapshots for u128.
97403b1
@BriocheBerlin
Add impls for i128 and u128.
11b0e71
@BriocheBerlin
Test {:?} format specifier.
c13f676
@Lotterleben
Copy link
Contributor

Awesome, feel free to squash and merge!

@BriocheBerlin BriocheBerlin merged commit 497cc50 into main Dec 3, 2020
@BriocheBerlin BriocheBerlin deleted the knurling-rs/gh199 branch December 3, 2020 09:41
@Lotterleben Lotterleben mentioned this pull request Dec 3, 2020
Closed
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
No one assigned
Labels
breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version
Projects
None yet
Milestone
v0.2.0
Development

Successfully merging this pull request may close these issues.

impl Format for i128/u128
5 participants
@BriocheBerlin @Lotterleben @japaric @jonas-schievink @justahero