Copter, Plane: persist the fixed notch conversion to INS_HNTC2 - #33878
Open
andyp1per wants to merge 2 commits into
Open
Copter, Plane: persist the fixed notch conversion to INS_HNTC2#33878andyp1per wants to merge 2 commits into
andyp1per wants to merge 2 commits into
Conversation
The conversion of the pre-4.2 fixed notch set INS_HNTC2_MODE and INS_HNTC2_HMNCS as defaults, gated on the notch being disabled. A default is not stored, so the gate closed as soon as INS_HNTC2_ENABLE was saved and both parameters silently reverted to their table defaults on the next boot: a converted fixed notch became a throttle tracking notch on all harmonics, and a user configuring HNTC2 saw MODE change from 0 to 1 across a reboot. Save the values instead, and only when an old fixed notch was actually converted.
The conversion of the pre-4.2 fixed notch set INS_HNTC2_MODE and INS_HNTC2_HMNCS as defaults, gated on the notch being disabled. A default is not stored, so the gate closed as soon as INS_HNTC2_ENABLE was saved and both parameters silently reverted to their table defaults on the next boot: a converted fixed notch became a throttle tracking notch on all harmonics, and a user configuring HNTC2 saw MODE change from 0 to 1 across a reboot. Save the values instead, and only when an old fixed notch was actually converted.
3 tasks
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
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.
Summary
INS_HNTC2_MODEandINS_HNTC2_HMNCSare applied as defaults inside the pre-4.2 fixed-notch conversion, gated on the notch being disabled. Defaults are not stored, so the gate closes the momentINS_HNTC2_ENABLEis saved and both parameters revert to their table defaults on the next boot. Two consequences: a vehicle upgrading from 4.1 gets its converted fixed notch turned into a throttle-tracking notch on all harmonics one reboot later, and anyone setting HNTC2 up as a static notch sees MODE go 0 -> 1 across a reboot and has to set it explicitly to make it stick.Save the values instead, and only when an old fixed notch was actually converted.
Classification & Testing (check all that apply and add your own)
SITL A/B against a synthesised pre-4.2 eeprom holding the old fixed notch (
INS_NOTCH_ENABLE=1 ATT=30 FREQ=95 BW=25):Separately, on a wiped eeprom: enabling
INS_HNTC2_ENABLEand rebooting used to moveINS_HNTC2_MODEfrom 0 to 1 andINS_HNTC2_HMNCSfrom 1 to 3. Both are now stable across reboots.Description
The intent of the two
set_default_by_name()calls was to make a notch converted from the old fixedINS_NOTCH_come up as a static notch on the first harmonic. That intent is correct, but expressing it as a default makes the effective default depend on the runtime value of another parameter, and it is never persisted, so it evaporates as soon as the conversion savesINS_HNTC2_ENABLE.This deliberately does not make
INS_HNTC2_MODEdefault to 0 in general. That would match what a GCS shows while the notch is off, and it would let adefaults.parmconfigure a static HNTC2 withENABLEalone, but it would also silently switch every vehicle currently flying HNTC2 with an unsaved MODE from throttle tracking to static. The documented default has always been 1, since the parameter tables are whatparam_parsereads, so this makes behaviour match the documentation rather than changing anyone's filter configuration in the field.