-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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.
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
samples: maxim_ds3231: increase variable size in min_alarm_handler #65091
samples: maxim_ds3231: increase variable size in min_alarm_handler #65091
Conversation
Hello @CharlesDias, and thank you very much for your first pull request to the Zephyr project! |
While the fix looks good (thanks!) I am confused by the printk? It's printing "... seconds.us" so the millisecond part is lost, is it not? |
Ah, nevermind, it's the |
Yes, you right! It would be better to change it to ms. Should I add a new commit to change its name, or just add in the previous one and force it? |
Good question :) I think it makes sense to have everything squashed into one commit as part of the same "getting milliseconds right" effort. And then force push indeed. Thanks! |
Change the variable name from **us** to **ms** and increase its size to **uint16_t** within the min_alarm_handler function. This update allows the function to correctly handle the millisecond values in the range of 0 to 999. Signed-off-by: Charles Dias <charlesdias.cd@outlook.com>
69f0718
to
a27afb0
Compare
Done. Thanks! |
Hi @CharlesDias! To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge. Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁 |
Change the variable us from uint8_t to uint16_t within the min_alarm_handler function. This update allows the function to correctly handle and print values in the range of 0 to 999.