Skip to content

Commit

Permalink
fix delayed save + exclude manual mode
Browse files Browse the repository at this point in the history
  • Loading branch information
@breadoven
breadoven committed Oct 23, 2023
1 parent 013de45 commit b6b5a98
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 2 deletions.
5 changes: 4 additions & 1 deletion src/main/fc/fc_core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,10 @@ void taskMainPidLoop(timeUs_t currentTimeUs)
if (!ARMING_FLAG(ARMED)) {
armTime = 0;

processDelayedSave();
// Delay saving for 0.5s to allow other functions to process save actions on disarm
if (currentTimeUs - lastDisarmTimeUs > USECS_PER_SEC / 2) {

This comment has been minimized.

Sign in to view

Copy link
@MrD-RC

MrD-RC Jan 30, 2024

Collaborator

We should not be delaying this. Any save on disarm should use the same mechanism, so there is only a single save event. That was the whole point of it. Which save actions were also being called on disarm? The only two I was aware of was stats and continuous servo trim.

This comment has been minimized.

Sign in to view

Copy link
@MrD-RC

MrD-RC Jan 30, 2024

Collaborator

Ah, the comment was confusing. It's to wait for actions to complete storing data in settings that are saved. Not for other save processes to happen. I didn't think it should have been needed, as it happens on the next loop. But I guess with the schedulers etc. Some things haven't finished processing when the save is called. All good. Though I may update the comment.

This comment has been minimized.

Sign in to view

Copy link
@breadoven

breadoven Jan 31, 2024 •
edited
Loading

Author Collaborator

The problem was actually to do with the delayed save also reloading the configuration as part of the post save verification. This overwrote the (adjusted) Auto Level Trim value with the original config setting value before the config setting value (mutable value) had been updated by the Auto Level Trim function (function updates slower than loop rate). There's probably a better way of doing this but just delaying the save was the easiest short term solution.

This comment has been minimized.

Sign in to view

Copy link
@MrD-RC

MrD-RC Jan 31, 2024

Collaborator

It’s cool. I realised what was going on when I found the PR and had a better look at the code surrounding it.

I am changing it slightly in #9681 But only by passing the result of the IF to the function. Just so that a waiting to save message can be displayed, and not blocked by the delay.
processDelayedSave();
}
}

#if defined(SITL_BUILD)
Expand Down
3 changes: 2 additions & 1 deletion src/main/flight/pid.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1263,7 +1263,8 @@ pidBank_t * pidBankMutable(void) {
bool isFixedWingLevelTrimActive(void)
{
return IS_RC_MODE_ACTIVE(BOXAUTOLEVEL) && !areSticksDeflected() &&
(FLIGHT_MODE(ANGLE_MODE) || FLIGHT_MODE(HORIZON_MODE)) && !FLIGHT_MODE(SOARING_MODE) &&
(FLIGHT_MODE(ANGLE_MODE) || FLIGHT_MODE(HORIZON_MODE)) &&
!FLIGHT_MODE(SOARING_MODE) && !FLIGHT_MODE(MANUAL_MODE) &&
!navigationIsControllingAltitude();
}

Expand Down

1 comment on commit b6b5a98

@MrD-RC
Copy link
Collaborator

@MrD-RC MrD-RC commented on b6b5a98 Jan 30, 2024

Choose a reason for hiding this comment

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

@breadoven I don't think adding a delay is the right way to deal with this. The whole point of the processDelayedSave mechanism was for a single save event on disarm. Adding a delay, so that other things can save on disarm is counter intuitive.

Please sign in to comment.