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

Fix charge_rate when discharging #2

Merged
merged 1 commit into from
Dec 11, 2023
Merged

Fix charge_rate when discharging #2

merged 1 commit into from
Dec 11, 2023

Conversation

bencochran
Copy link
Contributor

@bencochran bencochran commented Dec 9, 2023 •
edited by eldruin
Loading

As reported in #1, MAX17048 & MAX17049 yield negative CRATE values when discharging, so interpret them as i16 instead of u16 before converting to f32.

(I’m relatively new to Rust so feel free to point me towards a different idiom to use here if you’d prefer)

Fixes #1

eldruin
eldruin reviewed Dec 9, 2023
View reviewed changes
Copy link
Owner

@eldruin eldruin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice, thank you!
Could you also adapt the tests (you can run them with cargo test on linux) and add an entry to the changelog?

@bencochran
Copy link
Contributor Author

Done! Let me know if you’d like to see any other changes.

@eldruin
Copy link
Owner

eldruin commented Dec 9, 2023

Thank you!
Sorry, I had to update a few things.
Could you rebase this to master and also run cargo fmt?

@bencochran
Fix charge_rate when discharging
6b2510d 
MAX17048 & MAX17049 report negative `CRATE` values when discharging, so interpret them as i16
instead of u16 before converting to f32.

Resolves eldruin#1
@bencochran
Copy link
Contributor Author

Ah, forgot to fmt after the tests. Still getting my habits in order. All rebased and formatted now.

Thanks for getting builds working again!

eldruin
eldruin approved these changes Dec 11, 2023
View reviewed changes
Copy link
Owner

@eldruin eldruin left a comment

Choose a reason for hiding this comment

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

Nice, thank you!

@eldruin eldruin merged commit 0ac8591 into eldruin:master Dec 11, 2023
31 checks passed
@bencochran
Copy link
Contributor Author

Thanks! And thanks for the library!

@eldruin
Copy link
Owner

eldruin commented Dec 11, 2023

You are welcome. Thanks to you too.
I have published this fix in release 0.1.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eldruin eldruin eldruin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Charge rate reports wrong values
2 participants
@bencochran @eldruin