Menu Options in Sysconfig - #121
Merged
Merged
Conversation
The menu had three pages; it now has two. SD LOGGING is gone. Nothing received it: there is no SD controller task, SD_CONTROLLER_TASK_ENABLE is 0 and the queue is NULL in main.c, so every publish path was already compiled out. Removed the page, the FSM state and flag, SessionController::PublishSdLoggingChange, the display screen id and its message field, and the layout branches and tests. The sd_controller queue and TaskMonitor slot stay -- those are the future task's seat at the table, not part of the menu option. PID LOGGING is renamed PID CONTROL, which is what it does: it gates arming the loop in-session, not logging. PID_ENABLE and PID_DESIRED_RPM become runtime sysconfig parameters (ids 34 and 35, appended -- no renumbering, so no protocol bump). The FSM no longer keeps copies: its getters read the store and its handlers write it, so the encoder and the host edit one value rather than two that drift. The compile-time PID_CONTROLLER_TASK_ENABLE stays alongside them and means something different -- whether the task exists at all, versus whether the SessionController offers it -- and config.h now spells that out. Three PID defects fixed, all pre-existing rather than regressions from #119: - The first sample after every enable slammed the brake to full. Reset() zeroes _prevTimestamp while the next sample carries a live microsecond counter, so GetTimeDelta returned time-since-boot and the integral took a term the size of the timestamp range on pass one. K_I defaults to 1.0f, so this fired every time. Guarded with _havePreviousSample: the first sample sets the baseline and drives nothing. A flag rather than seeding the timestamp, because a sample predating the reset hit the same bug through GetTimeDelta's wrap branch. - The loop stayed armed after a session ended, so the task ran on against a finished session until every pass logged a queue-full warning -- and with _prevPIDEnabled stuck true the next session found no enable edge, never pointed the BPM at the PID output, and showed PIDE over a brake the controller no longer reached. ShowSessionScreen disarms on entry; PublishSessionTransition publishes the disable on the way out, which is the only place that can, since Run()'s PID step sits below the in-session gate. - Clamp() had been dead since #119 removed the output mixing. Setpoint changes now reach a running loop too: PublishPidEnableChange becomes PublishPidInstruction and republishes when either half moves. It only ever fired on an enable edge, which was enough while the menu was the sole editor and unreachable mid-session -- but the host can move the setpoint over USB now. Verified: ARM Debug build clean with no warnings, 105/105 firmware host tests, 266/266 Dyno.Core tests, both message_gen drift checks in sync. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Writing SYSCFG_PID_ENABLE or SYSCFG_PID_DESIRED_RPM from the app left the LCD showing the old value while sitting on the settings menu. The store is plain RAM and USB_CMD_SET_SYSCONFIG is applied by the USB task itself, deliberately with no queue and no task wake-up -- so nothing told the FSM the value had moved. Every other route to the panel is an FSM event that reposts on its way through, which is why an encoder tick always redraws; a host write has no such event. Making these two settings host-writable in the previous commit is what opened the gap, since until then the menu was the only writer. RefreshHostEditedSettings polls for it instead: PostDisplayState now records what it sent for the two, and the SessionController calls the check once per pass, above the in-session gate because the settings pages only exist outside a session. It compares two words and posts nothing when they match, so a steady state still produces no queue traffic. Verified: ARM Debug build clean, 105/105 firmware tests, 266/266 Dyno.Core tests, schemas in sync. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ttern Follows the force sensor's ReconcileConfig discipline properly. Its ApplyIfChanged leaves _applied stale when an I2C write fails, so the next reconcile retries that register; the display reconcile was missing the equivalent. PostDisplayState posts with timeout 0 -- a full queue drops the frame rather than stalling the SessionController -- but it recorded the "what is on the panel" pair regardless of whether the put took. A dropped frame therefore marked the value as shown when it had never reached the panel, and since the reconcile is a diff against that pair, it would never look at it again: the stale reading stuck until some unrelated event reposted. Now the pair is only updated once osMessageQueuePut returns osOK. Renamed RefreshHostEditedSettings to ReconcileHostEditedSettings to match the name the rest of the board uses for this. No bounded-wait wakeup is needed on this side, unlike the force sensor's FORCESENSOR_COMMAND_POLL_OSDELAY: that exists because its task would otherwise sit in osWaitForever on the enable queue while idle, whereas SessionController::Run ends every iteration on osDelay and so is already awake to reconcile 100 times a second. Verified: ARM Debug build clean, 105/105 firmware tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.