Stats is not being saved when Rate profile adjustments are set (latest Master)

[limonspb]

Describe the bug

If Adjustments are set to change rate profiles on a switch, then stats (stats_total_flights) is not being saved on disarm and goes back to the previous value after the power cycle (4.3 branch)

To Reproduce

1. Make sure you can arm the quad, props off.
2. set stats_min_armed_time_s = 5
   set stats_total_flights = 1
   save
3. set adjustments, for example like that, save:
   
4. unplug USB, plug the battery
5. Arm your quad, keep it armed for 5+ seconds, disarm
6. OSD flights counter will do +1 - good (if you have it set)
7. Unplug the battery, plug it back.
8. Check stats_total_flights via OSD or CLI - it will show the old value, which is 1 in this case.

Expected behavior

stats_total_flights must become = 2
and it should stay = 2 even after the power cycle.

Flight controller configuration

diff_bug.txt

resource show all:
resource_show_all_bug.txt

Setup / Versions

• Flight controller: Talon F7 V2, but the bug exists also with Pyrodrone F7 and supposedly all other FCs

• Other components: crossfire receiver, talon ESC.

All wired according to the diagram and performs/flies as expected except the stats not being saved.

[adarkthinline]

I was touching my goggles and noticed.... that it is an issue. Talon F7 V2 and Pyro F7 both FC will not keep the counter unless the BF adjustments is turned off.

[haslinghuis]

We should check setConfigDirty() in fc/ rc_adjustments.c
What was the last version that did work?

For reference stats were added here: #7927

[limonspb]

I tried Master from a month ago - the same result.
It's kind of a corner case using rates on a switch AND stats, i guess.

[haslinghuis]

Please someone check flashing other releases. Start with 4.2. If it works check 4.2.5. If it work's go up, if it doesn't go down.

[limonspb]

@haslinghuis i think you are right... Is it kind of a safety feature that prevents saving stats?
Should we remove it?

[haslinghuis]

Agree. But it's an expensive operation. So we just have to make sure the dirty flag is set.

[limonspb]

@haslinghuis this writeStats(...) function is happening only when stats counter was incremented and only within stats.c.

[haslinghuis]

In that case it would be safe to remove the condition here but you have to check it.

[limonspb]

@haslinghuis as far as I understand this condition serves for 1 reason here. This scenario. Imagine no stats is set and:

1. plug your quad
2. Change PIDs/rates via OSD menu of via switches
3. Don't save - exit no save
4. Fly, land, disarm (with new PIDs/Rates)

At this point a user has an option - he can go ahead and save changes via OSD
or he can unplug if he does not want these changes to be saved.

Now if we introduce stats without checking that condition, it will automatically save everything on disarm including new PIDs Rates etc.

[adarkthinline]

I have used this adjustments setup (see image below) for my 3-way position switch to change between rate profiles. It has been setup the same way for these versions of BetaFlight. BF 3.5.7, - BF 4.2.2 and now in development BF 4.3. Currently in BF 4.3 I added the pack counter in CLI and OSD. I tested it (plug in battery, arm, wait 5+ sec, disarm, check stats, counter value increased by 1, re-arm the quad --- repeat). The count value would increase by 1 each test cycle "IF" the battery remained plugged into my quad. "IF" you disconnect the battery and plug in again the pack counter will reset to zero.

When the adjustments are removed the counter works properly.

---

I found this video by Joshua Bardwell...
"Betaflight 4.1 Rate Profile with Jumper T16 6-position switch" https://www.youtube.com/watch?v=F6xTZkAfpR0

In it he states to setup adjustments like so... (see image below). He also states something along the line that this is the NEW way to setup your adjustments to a switch at least moving forward from 4.1. I did set it up as seen below and tested again as stated above.

The pack counter now works properly.

---

I am only speculating, but it seems to me that if there was a change in the code, at some point where the code recognized the switch as a 3-position or 6-position, then it designated the function instead of it needing to be programmed by the user into their radio. If this is the case then by attempted to designate an adjustment for each switch position it is causing an internal conflict.

[mikeller]

@limonspb: This is a known limitation, and as such I'd classify this as 'by design', and as a 'fix' I think an update of the documentation around statistics is the right approach.
A couple of considerations here:

• assuming that most users fly their craft a lot more than they tune it, the occurrence of this is most likely and edge case;
• the negative impact of having an unwanted configuration saved probably outweighs the negative impact of having inaccurate stats recorded - other events such as crashing / loss of power without disarming first will have the same effect;
• stats are meant to convey a rough idea, and not pinpoint accuracy.

[limonspb]

@limonspb: This is a known limitation, and as such I'd classify this as 'by design', and as a 'fix' I think an update of the documentation around statistics is the right approach.
A couple of considerations here:

• assuming that most users fly their craft a lot more than they tune it, the occurrence of this is most likely and edge case;
• the negative impact of having an unwanted configuration saved probably outweighs the negative impact of having inaccurate stats recorded - other events such as crashing / loss of power without disarming first will have the same effect;
• stats are meant to convey a rough idea, and not pinpoint accuracy.

I agree with your statements, @mikeller .
However, @adarkthinline managed to make it work with the different settings for the adjustments... THat one I can't understand :) He described that in his last long message.

[AlexandreBonneau]

I just got hit with this bug. And as an fpv noob, I think this case must happen pretty often;
I got a new quad, and instead of trying some rates, landing it, unplugging everything, plugging it in betaflight-configurator, change the rate, re-plug it, rinse and repeat, I used the rate profile switch to test 3 different ones at once.

Well, the stats are not saved indeed.

What's weird to me is that on my other quads I could run set stats = ON, but on this Mobula7 1S, the stats variable does not exist (I flashed that version on it : # Betaflight / CRAZYBEEF4SX1280 (HAMO) 4.3.0 Apr 6 2022 / 07:51:10 (2fd2e8eca) MSP API: 1.44).

It seems to me (still as an fpv newbee), that stats are not settings, they are just...stats. So saving them should not mean saving a configuration, isn't? Perhaps stats should be saved as 'stats data' separated from the configuration settings?

[haslinghuis]

@limonspb can we close this?

[limonspb]

@limonspb can we close this?

pretty sure yes.