Add stats_total_packs

[bree29]

stats_total_packs

What is there today :

stats_total_flights is supposed to count our flights. And it does ! But what do we count as "flights" ? (Turtle mode, breaks, fast checkin, ...). Reviewing DVR (and the ones we forgot to press play), we have to go to the end to see which flight we are talking about. Sometimes we didn't really miss a DVR : flight count for two.

What's that feature provides :

That new feature works just as stats_total_flights, except :

• it does not count the times you've satisfied minArmedTimeS, that can be more than one time in a pack if you disarm and arm again in the middle of your pack
• it does count the times you've satisfied minArmedTimesS, and only once per pack. Once it has be incremented one time, it won't count that "second flight".

I both add this feature and the ability to add it in OSD both stat and during the flight. It is usefull for "enthousiastic racers" (see #10215).

Issues ?

However, I am concerned about adding yet another persistant_stat element. There is today 26 elements on 32 availables on the 32bit chip. See #10503 arguments.

It would be possible to set stats_total_flights to either work like right now, or counting in packs. It would also be trickier, "and why change something that does work / field test", and was suggested in #10803 to make it a different parameter.

Fixes #10803

The commit has passed the make pre-push and I attach unified_zip targets for user to test the feature.

2021-06-22_17-52 : updated unified_zip from last commit.
betaflight_4.3.0_STM32G47X_68549ae7f.zip
betaflight_4.3.0_STM32F745_68549ae7f.zip
betaflight_4.3.0_STM32F7X2_68549ae7f.zip
betaflight_4.3.0_STM32F411_68549ae7f.zip
betaflight_4.3.0_STM32F405_68549ae7f.zip

[hydra]

Hey, the idea of recording total packs is an interesting one, however there is an issue in that it will also increase the counter incorrectly if you're testing with only a USB cable plugged in and no battery.

You can check to see if a battery is connected by checking the detected cell count. I think it might be better if you check the battery cell count > 0 before saving.

It might also make sense to check total flight time on the current pack is greater than a sensible value, e.g. 1 minute.

[haslinghuis]

Please add the elements in the enum as last before the counter and increase the PG version.
Instructions on how to do that are mentioned in the source code.

[limonspb]

@haslinghuis should we put a label "Needs coordination with betaflight-configurator"?

[bree29]

@limonspb

should we put a label "Needs coordination with betaflight-configurator"?

I was planning to flash 4.3 with stats_total_packs next week and build Configurator with my edits and see how it's going in the next days, so I can test this weekend.

I've already commit on my branch. I want to test before PR

bree29/betaflight-configurator@b82438e

[asizon]

@limonspb this is a new feature so coordination with configurator is not needed, also with our msp rules we need to wait for the next release for add it to MSP.

[limonspb]

@limonspb this is a new feature so coordination with configurator is not needed, also with our msp rules we need to wait for the next release for add it to MSP.

This change actually changes OSD_ITEM_COUNT.
So if we merge it without configurator modifications, we will see an "unknown" OSD element there.
I think we should merge it together with adding this feature to the configurator.

[asizon]

@limonspb you are right I overloocked osd elements parts sorry, tag should be added yes.

[bree29]

Tested on STMF7X2 ; forgot to set In OSD and post flight but according to CLI it looks like it works.

I will do more testing next week, since my testing sessions were interrupt by a running dog that didn't want to leave me alone. :'(

One thing is wrong for sure though : OSD TOTAL PACKS in flight, just like the OSD TOTAL FLIGHTS, will print the #count of the previous flight, not the current, since it will be incremented only at disarm.

I'll be looking into it. Should I make another PR to correct OSD TOTAL FLIGHTS behavior (show stats_total_flights + 1) since it is related to another feature ?

[haslinghuis]

Awaiting more tests and insight - a PR has to be atomic (not depending on other PR's). First have to see how new insights and testing influences stats_total_flight's case on count and disarm.

[bree29]

Made the corrections needed. Just edited the osdelement.c so OSD show the pack we're on.

Tested on workbench with 20 secs arming sequences on STM32F7X2. It's working fine :

• the pack count increment only after first successful arm time (20 sec in my case)
• the pack count does not increment itself on the next disarmings sequences ; while flights count does.
• arm time has to be satisfied (less than 20sec, as set in mine, will not increment either of these
• adding "+1" in osdelement.c allowed to show in OSD the current pack : the one that will show in the post flight stat if the flight is sucessfull.

---

The current feature, as intended, does not seem to influence how stats_total_flights works. If you will, I can make the same little trick that I did with total_packs var in osd_elements.c (+1) in the same PR or in another, so everything would be more coherent.

[asizon]

Rebase needed here @bree29

[bree29]

No way to figure it out.

Auto-merging src/main/io/serial.c
CONFLICT (content): Merge conflict in src/main/io/serial.c
Auto-merging src/main/io/serial.h
Auto-merging src/main/target/NUCLEOH743/target.h
Auto-merging src/main/target/common_defaults_post.h
error: could not apply 263c5fa37... Add UART9/10 support.
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 263c5fa37... Add UART9/10 support.

Same story on the configurator.

Tag them for 4.4 ; you'll have plenty of work right now ; I'll try to get a Git wizard somehow.

[ctzsnooze]

Usually the best solution is to first squash all your commits into one, then do the rebase against master. I find GitHub desktop very useful.

[bree29]

Well, there is my issue. I could revert back to my initial commit. I solved all conflicts that have been raised before (mainly beautify codes) by keeping upstream and discarding manually my change. I then get my cleanish commit.

But everytime I try rebasing against master with git pull --rebase origin master and push it push origin +stats_total_packs, it gets people's commit into my PR and here am I with more than 1 commit in the pull request.
(and now, with conflicting files I never touched before). It really tires me. And since I rebase, I'm not sure how I should do to revert HASH to my commit, since now its says mine is last)

[ctzsnooze]

Shouldn't you rebase against upstream master?

If your 'cleanish commit' is OK, there are not a ton of changes. You could just get master and manually apply them with copy and paste, if nothing else worked.

[limonspb]

@bree29 my suggestions, to sort of start over from clean:

0. Copy your local repo folder for backup purposes
1. remember the hash of your feature commit - save it somewhere for later. I believe its 7b5516f5a4cb093870f1f3ca8999fab048e9bacc - please double check it
2. git checkout master - switch to master
3. git branch -D stats_total_packs - remove your local branch
4. git fetch upstream master - get updates from upstream
5. git merge upstream/master - make your local master up to date with the upstream master
6. git checkout -b stats_total_packs - recreate your local branch and switch to it
7. git cherry-pick 7b5516f5a4cb093870f1f3ca8999fab048e9bacc - put your commit on top of stats_total_packs
8. resolve conflicts and run git cherry-pick —continue
9. git push origin stats_total_packs -f - force push your branch, it will update your PR too.

:)

[bree29]

I do think I made it (this time). I see only one commit here. Thanks for my copypasta monkey mind, it helped a lot, really.

[blckmn]

AUTOMERGE: (FAIL)

• github identifies PR as mergeable -> FAIL
• assigned to a milestone -> PASS
• cooling off period lapsed -> PASS
• commit count less or equal to three -> PASS
• Don't merge label NOT found -> PASS
• at least one RN: label found -> PASS
• Tested label found -> FAIL
• assigned to an approver -> FAIL
• approver count at least three -> FAIL

[haslinghuis]

@bree29 please rebase

[github-actions]

Do you want to test this code? Here you have an automated build:
Assets
WARNING: It may be unstable. Use only for testing! See: https://www.youtube.com/watch?v=I1uN9CN30gw for instructions for unified targets!

[ctzsnooze]

@limonspb, @bree29...

To be honest, I think we should not add a new element for 'packs'.

The value of the parameter is only for the pleasure of the pilot, to see how much time this particular quad has been flown since the stats we're erased. It is not a high value thing. Total time in the air is likely more useful than 'packs'.

Hence my thinking is that perhaps we could revise the meaning of 'flights', or how we record total flight time, but that we should not continue with this PR, and it should be closed.