Skip to content

Reorganise stats screens#9617

Merged
MrD-RC merged 41 commits intomasterfrom
MrD_Reorganise-Stats-screens
Apr 1, 2024
Merged

Reorganise stats screens#9617
MrD-RC merged 41 commits intomasterfrom
MrD_Reorganise-Stats-screens

Conversation

@MrD-RC
Copy link
Member

@MrD-RC MrD-RC commented Jan 4, 2024 •
edited
Loading

This started as a basic re-organisation of the stats screens. But kinda grew a little. The main result is still a slightly more organised stats screen. But it adds a few more stats. Also, where things are now separated, it will be easier to make stats customisable.

Here's an example of the stats screen in Avatar (HITL) from 2 sequential flights.
Stats - 2 sequential flights

Here are examples of the analogue stats screen. Note, that I have updated the AVG EFFICIENCY F/T to fit.
image image

Bug Fixes

Not all stats were resetting on arm. As these are flight stats, this should be the case. I have gone through and made sure that all stats now reset when armed. But, not reset after an in-flight re-arm. The exception is the energy used from the battery. It is understandable that this stat would be useful if it were persistent. So both are shown. Total from when the battery was plugged in and the amount since arming.

New data on the stats screen

  • Min Volts now shows both pack and cell voltage
  • Used energy now shows total and flight use
  • Average efficiency also shows total and flight energy efficiency
  • Min and Max satellites
  • Min and Max ESC temperature
  • If using a non-metric OSD. The metric efficiency can be shown by enabling set osd_stats_show_metric_efficiency = on
  • Blackbox file number

Other things of note

  • osd_stats_min_voltage_unit has been removed, as both battery and cell voltages are shown now
  • Some stats titles have been subtly renamed, to make them easier to understand and fit better on screen:
    • Fly Time -> Flight Time
    • Traveled (sp) Distance -> Flight Distance
    • Max Distance -> Distance from home
    • Min Battery Volt -> Min Volts Pack/Cell
    • Used Capacity -> Used Energy Flight/Total
  • The headers and setting save status messages are now centred

Requires Configurator iNavFlight/inav-configurator#1941

@MrD-RC MrD-RC added this to the 7.1 milestone Jan 4, 2024
@MartinHugh
Copy link

MartinHugh commented Jan 4, 2024

Very nice update. I see the blackbox filename is available too - very useful.
Is the proposal to encode part of the model name in the blackbox filename deferred for a later update?

@MrD-RC
Copy link
Member Author

MrD-RC commented Jan 4, 2024 •
edited
Loading

Seems there is a discrepancy in compilers. The build is failing with
image

Yet my compiler thinks it should be a long
image

So which is correct. Should an int32_t be a long integer or integer.

EDIT
Looks like the non-MAC build routines also think it should be a long, not an int??
image

@stronnag @DzikuVx any ideas?

stronnag
stronnag reviewed Jan 5, 2024
View reviewed changes
Copy link
Collaborator

@stronnag stronnag left a comment

Choose a reason for hiding this comment

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

Just a comment ...
I really don't like copying buff onto itself, when instead one could:

 tfp_sprintf(buff+strlen(buff), " %c%05" PRId32 " ",  SYM_BLACKBOX, logNumber);

Couldn't you? For the same effect.

@MrD-RC
Copy link
Member Author

MrD-RC commented Jan 7, 2024

Just a comment ... I really don't like copying buff onto itself, when instead one could:

 tfp_sprintf(buff+strlen(buff), " %c%05" PRId32 " ",  SYM_BLACKBOX, logNumber);

Couldn't you? For the same effect.

Done. I found a few other occurrences throughout the OSD.c file.

@MrD-RC MrD-RC mentioned this pull request Jan 7, 2024
Closed
@MrD-RC MrD-RC marked this pull request as ready for review January 7, 2024 21:36
@MrD-RC MrD-RC requested a review from stronnag January 7, 2024 21:40
stronnag
stronnag reviewed Jan 8, 2024
View reviewed changes
stronnag
stronnag reviewed Jan 8, 2024
View reviewed changes
stronnag
stronnag reviewed Jan 8, 2024
View reviewed changes
Copy link
Collaborator

@stronnag stronnag left a comment

Choose a reason for hiding this comment

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

Couple of comments, one trivial / pedantry, the other needs at least a second opinion.

@MrD-RC MrD-RC requested a review from stronnag January 8, 2024 20:17
@MrD-RC MrD-RC added the Release Notes Add this when a PR needs to be mentioned in the release notes label Jan 10, 2024
@DzikuVx
Copy link
Member

DzikuVx commented Jan 12, 2024

@MrD-RC We can't push it to 7.1. The change in osd_stats_min_voltage_unit is a breaking change comparing to 7.0.

@DzikuVx DzikuVx modified the milestones: 7.1, 8.0 Jan 12, 2024
@DzikuVx DzikuVx changed the base branch from release_7.1.0 to master January 12, 2024 09:24
@MrD-RC
Copy link
Member Author

MrD-RC commented Jan 12, 2024

@MrD-RC We can't push it to 7.1. The change in osd_stats_min_voltage_unit is a breaking change comparing to 7.0.

I was wondering about that. But, I figured that both options for osd_stats_min_voltage_unit are now displayed. It wasn’t so much of an issue. But yes, it will cause an error in the diff.

@DzikuVx
Copy link
Member

DzikuVx commented Jan 12, 2024

Then agreed. @MrD-RC please resolve conflicts and let's merge to master waiting for INAV 8

MrD-RC added 3 commits January 13, 2024 09:21
@MrD-RC
Updates
be9da9a 
- Small update for analogue
- Removed all instances of tfp_sprintf overwriting the buffer with itself.
@MrD-RC
Removed round brackets from analogue stats headers
c48f514
@MrD-RC
Update osd.c
ac74878 
- fixed buffer issue
- agreed to pedantry ;)
@MrD-RC MrD-RC force-pushed the MrD_Reorganise-Stats-screens branch from 4ded247 to ac74878 Compare January 13, 2024 09:21
MrD-RC added a commit to iNavFlight/inav-configurator that referenced this pull request Jan 13, 2024
@MrD-RC
Add blackbox element to osd page
d256a42 
Accompanies iNavFlight/inav#9617
@MrD-RC MrD-RC mentioned this pull request Jan 13, 2024
Merged
@MrD-RC
Copy link
Member Author

MrD-RC commented Jan 13, 2024

Very nice update. I see the blackbox filename is available too - very useful. Is the proposal to encode part of the model name in the blackbox filename deferred for a later update?

I'll change that separately.

@MrD-RC MrD-RC requested a review from DzikuVx January 13, 2024 10:45
@MrD-RC
Merge branch 'master' into MrD_Reorganise-Stats-screens
e1142c8
@MrD-RC
Show stats if space & improve formatting
57d84fc 
- Show `stats` if enabled and space allows on the post flight statistics
- Improve the layout of some elements
@MrD-RC
Corrected required remaining rows for stats check
014942a
@MrD-RC
Copy link
Member Author

MrD-RC commented Feb 17, 2024

image
Now with added stats

@MrD-RC
Formatting changes
dd9e459 
- Removed blank spaces from data, to make it look neater
- Current and Power are now on a single row
@MrD-RC
Commented out currently unused function
5aa15a5
@b14ckyy
Copy link
Collaborator

b14ckyy commented Mar 20, 2024

I guess also missed to be merged and got a few new conflicts @DzikuVx

@MrD-RC
Copy link
Member Author

MrD-RC commented Mar 20, 2024

I guess also missed to be merged and got a few new conflicts @DzikuVx

I’ve been holding off merging until 7.1 comes out and those changes are merged with master.

@MrD-RC
Temporarily remove the rearm period
9b94b4c 
This is causing problems with MSP DisplayPort. I hope to fix the issue and reinstate it in #9688
@MrD-RC
Copy link
Member Author

MrD-RC commented Mar 29, 2024

I need to test in flight on an MSP DisplayPort platform and disarm in flight. If that all goes ok, this will be good to merge.

@MrD-RC
Copy link
Member Author

MrD-RC commented Mar 31, 2024

Tested in flight with MSP DisplayPort. It worked as expected.

@MrD-RC MrD-RC merged commit 9f5119c into master Apr 1, 2024
@MrD-RC MrD-RC deleted the MrD_Reorganise-Stats-screens branch April 1, 2024 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stronnag stronnag stronnag approved these changes

@DzikuVx DzikuVx Awaiting requested review from DzikuVx

Assignees

No one assigned

Labels

Release Notes Add this when a PR needs to be mentioned in the release notes Requires Configurator

Projects

None yet

Milestone

8.0

Development

Successfully merging this pull request may close these issues.

8 participants

@MrD-RC @MartinHugh @DzikuVx @b14ckyy @stronnag @druckgott @shota3527 @mluessi