Save mixer values to params when canned mixer is selected#448
Merged
gabesnow99 merged 1 commit intoremove-gnss-fullfrom May 21, 2025
Merged
Save mixer values to params when canned mixer is selected#448gabesnow99 merged 1 commit intoremove-gnss-fullfrom
gabesnow99 merged 1 commit intoremove-gnss-fullfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the mixer functionality so that when a canned mixer is selected, its inverted values are also stored in the firmware parameters.
- Adds two new methods, save_primary_mixer_params() and save_secondary_mixer_params(), to persist mixer values.
- Calls these new methods appropriately in the mixer initialization process.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/mixer.cpp | Introduces new parameter saving functions and calls them in init_mixing() to store the mixer values. |
| include/mixer.h | Declares the new save_primary_mixer_params() and save_secondary_mixer_params() methods. |
Comment on lines
+412
to
+444
| // This assumes the parameters are stored in order in the param enum | ||
| int output_param_index = (int) PARAM_PRIMARY_MIXER_OUTPUT_0 + i; | ||
| int pwm_rate_param_index = (int) PARAM_PRIMARY_MIXER_PWM_RATE_0 + i; | ||
| RF_.params_.set_param_int(output_param_index, primary_mixer_.output_type[i]); | ||
| RF_.params_.set_param_float(pwm_rate_param_index, primary_mixer_.default_pwm_rate[i]); | ||
| } | ||
|
|
||
| // Save the mixer values to the firmware parameters | ||
| for (int i=0; i<NUM_MIXER_OUTPUTS; ++i) { | ||
| // This assumes the parameters are stored in order in the param enum | ||
| int param_index = (int) PARAM_PRIMARY_MIXER_0_0 + 6 * i; | ||
| RF_.params_.set_param_float(param_index++, primary_mixer_.Fx[i]); | ||
| RF_.params_.set_param_float(param_index++, primary_mixer_.Fy[i]); | ||
| RF_.params_.set_param_float(param_index++, primary_mixer_.Fz[i]); | ||
| RF_.params_.set_param_float(param_index++, primary_mixer_.Qx[i]); | ||
| RF_.params_.set_param_float(param_index++, primary_mixer_.Qy[i]); | ||
| RF_.params_.set_param_float(param_index, primary_mixer_.Qz[i]); | ||
| } | ||
| } | ||
|
|
||
| void Mixer::save_secondary_mixer_params() | ||
| { | ||
| // Save the mixer values to the firmware parameters | ||
| // The secondary mixer does not have header values (they are the same as the primary mixer) | ||
| for (int i=0; i<NUM_MIXER_OUTPUTS; ++i) { | ||
| // This assumes the parameters are stored in order in the param enum | ||
| int param_index = (int) PARAM_SECONDARY_MIXER_0_0 + 6 * i; | ||
| RF_.params_.set_param_float(param_index++, secondary_mixer_.Fx[i]); | ||
| RF_.params_.set_param_float(param_index++, secondary_mixer_.Fy[i]); | ||
| RF_.params_.set_param_float(param_index++, secondary_mixer_.Fz[i]); | ||
| RF_.params_.set_param_float(param_index++, secondary_mixer_.Qx[i]); | ||
| RF_.params_.set_param_float(param_index++, secondary_mixer_.Qy[i]); | ||
| RF_.params_.set_param_float(param_index, secondary_mixer_.Qz[i]); |
There was a problem hiding this comment.
[nitpick] Consider refactoring the similar parameter saving loops in save_primary_mixer_params() and save_secondary_mixer_params() into a helper function to reduce code duplication and potential maintenance issues.
Suggested change
| // This assumes the parameters are stored in order in the param enum | |
| int output_param_index = (int) PARAM_PRIMARY_MIXER_OUTPUT_0 + i; | |
| int pwm_rate_param_index = (int) PARAM_PRIMARY_MIXER_PWM_RATE_0 + i; | |
| RF_.params_.set_param_int(output_param_index, primary_mixer_.output_type[i]); | |
| RF_.params_.set_param_float(pwm_rate_param_index, primary_mixer_.default_pwm_rate[i]); | |
| } | |
| // Save the mixer values to the firmware parameters | |
| for (int i=0; i<NUM_MIXER_OUTPUTS; ++i) { | |
| // This assumes the parameters are stored in order in the param enum | |
| int param_index = (int) PARAM_PRIMARY_MIXER_0_0 + 6 * i; | |
| RF_.params_.set_param_float(param_index++, primary_mixer_.Fx[i]); | |
| RF_.params_.set_param_float(param_index++, primary_mixer_.Fy[i]); | |
| RF_.params_.set_param_float(param_index++, primary_mixer_.Fz[i]); | |
| RF_.params_.set_param_float(param_index++, primary_mixer_.Qx[i]); | |
| RF_.params_.set_param_float(param_index++, primary_mixer_.Qy[i]); | |
| RF_.params_.set_param_float(param_index, primary_mixer_.Qz[i]); | |
| } | |
| } | |
| void Mixer::save_secondary_mixer_params() | |
| { | |
| // Save the mixer values to the firmware parameters | |
| // The secondary mixer does not have header values (they are the same as the primary mixer) | |
| for (int i=0; i<NUM_MIXER_OUTPUTS; ++i) { | |
| // This assumes the parameters are stored in order in the param enum | |
| int param_index = (int) PARAM_SECONDARY_MIXER_0_0 + 6 * i; | |
| RF_.params_.set_param_float(param_index++, secondary_mixer_.Fx[i]); | |
| RF_.params_.set_param_float(param_index++, secondary_mixer_.Fy[i]); | |
| RF_.params_.set_param_float(param_index++, secondary_mixer_.Fz[i]); | |
| RF_.params_.set_param_float(param_index++, secondary_mixer_.Qx[i]); | |
| RF_.params_.set_param_float(param_index++, secondary_mixer_.Qy[i]); | |
| RF_.params_.set_param_float(param_index, secondary_mixer_.Qz[i]); | |
| int output_param_index = (int) PARAM_PRIMARY_MIXER_OUTPUT_0 + i; | |
| int pwm_rate_param_index = (int) PARAM_PRIMARY_MIXER_PWM_RATE_0 + i; | |
| RF_.params_.set_param_int(output_param_index, primary_mixer_.output_type[i]); | |
| RF_.params_.set_param_float(pwm_rate_param_index, primary_mixer_.default_pwm_rate[i]); | |
| } | |
| // Save the mixer values using the helper function | |
| save_mixer_params(primary_mixer_, PARAM_PRIMARY_MIXER_0_0); | |
| } | |
| void Mixer::save_secondary_mixer_params() | |
| { | |
| // Save the mixer values using the helper function | |
| save_mixer_params(secondary_mixer_, PARAM_SECONDARY_MIXER_0_0); | |
| } | |
| void Mixer::save_mixer_params(const mixer_t& mixer, int base_param_index) | |
| { | |
| for (int i=0; i<NUM_MIXER_OUTPUTS; ++i) { | |
| int param_index = base_param_index + 6 * i; | |
| RF_.params_.set_param_float(param_index++, mixer.Fx[i]); | |
| RF_.params_.set_param_float(param_index++, mixer.Fy[i]); | |
| RF_.params_.set_param_float(param_index++, mixer.Fz[i]); | |
| RF_.params_.set_param_float(param_index++, mixer.Qx[i]); | |
| RF_.params_.set_param_float(param_index++, mixer.Qy[i]); | |
| RF_.params_.set_param_float(param_index, mixer.Qz[i]); |
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.
Motivation
This rosflight_ros_pkgs PR adds the functionality for other nodes to query
rosflight_iofor the current values of the firmware parameters.Previously, the canned mixer types were not stored in the firmware's parameters, so
rosflight_iohad no idea what the correct mixer parameters were when a canned mixer was selected.What I did:
save_secondary_mixer_paramsandsave_primary_mixer_paramsmethods to the firmware mixer. When a canned mixer is selected, it now inverts it like normal and then saves those values to the mixer parameters.Note that this is consistent with how custom mixers are saved -- the "inverted" version is saved to the parameters, which is then directly multiplied by the force vector to get the desired outputs.