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

Reorganise stats screens #9617

Merged
merged 41 commits into from
Apr 1, 2024
Merged

Reorganise stats screens #9617

merged 41 commits into from
Apr 1, 2024

Conversation

MrD-RC
Copy link
Collaborator

@MrD-RC MrD-RC commented Jan 4, 2024

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

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
Collaborator Author

MrD-RC commented Jan 4, 2024

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?

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
Collaborator 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 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
src/main/io/osd.c Outdated Show resolved Hide resolved
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
Collaborator 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

- Small update for analogue
- Removed all instances of tfp_sprintf overwriting the buffer with itself.
- 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
Copy link
Collaborator 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
- Show `stats` if enabled and space allows on the post flight statistics
- Improve the layout of some elements
@MrD-RC
Copy link
Collaborator Author

MrD-RC commented Feb 17, 2024

image
Now with added stats

- Removed blank spaces from data, to make it look neater
- Current and Power are now on a single row
@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
Collaborator 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.

This is causing problems with MSP DisplayPort. I hope to fix the issue and reinstate it in #9688
@MrD-RC
Copy link
Collaborator 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
Collaborator 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
15 checks passed
@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
Labels
Release Notes Add this when a PR needs to be mentioned in the release notes Requires Configurator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants