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.

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

Mavlink: round battery percentage up instead of down #8986

Merged
merged 1 commit into from
Mar 2, 2018

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Mar 1, 2018

I propse this change because of a user experience issue I found even myself running into almost every time: I set the low battery reaction to e.g. 15% in QGC through the parameter menu, test it while flying around looking at the battery percentage and when the percentage in QGC shows 15% I expect something to happen but it doesn't. It happens when QGC shows 14%.

Because:

  • The battery ratio still available is a float between 0 and 1 in the firmware for maximum accuracy.
  • The mavlink message is defined to only show integer (uint8_t) percentage for the battery level.
  • The mavlink receiver just assigns [unit8_t] = [float between 0 and 1] * 100.f which casts implicitly and rounds down.

It's mathematically all resonable but still not intuitive to the user like me or @potaito and I'm sure we're not alone. We expect the reaction to happen when the percentage is reached in terms of the integer we can see in the UI.

I think we need to fix the user experience no matter how it's done technically. We could als trigger the reaction with the rounded up number or similar. It just ssemed to me that the root cause is the rounding during conversion to MAVLink and that's why rounding up instead of down exactly where it happens is the go to solution.

FYI @Stifael

@MaEtUgR MaEtUgR self-assigned this Mar 1, 2018
Copy link
Contributor

@potaito potaito left a comment

Choose a reason for hiding this comment

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

@julianoes that is what you also wanted to change, right?

@dagar
Copy link
Member

dagar commented Mar 1, 2018

Why not round instead? roundf

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Mar 2, 2018

Why not round instead? roundf

Because then when the percentage switches from 16% to 15% you can again see no reaction but rather "in the middle" of e.g. 15%. Seeing the reaction on the expected percentage switch is the only reason why it even comes to my mind changing anything. I'm totally sure the pilot can not see any other difference if the percentage is rounded up or down or roundf.

@LorenzMeier
Copy link
Member

Makes sense from a UX perspective.

@LorenzMeier LorenzMeier merged commit 5bb9bab into master Mar 2, 2018
@potaito potaito deleted the battery-ui-round-up branch March 2, 2018 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants