Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mixer: FunctionMotors: leave NAN control values at NAN with non-zero THR_MDL_FAC #24108

Merged
merged 2 commits into from
Dec 13, 2024
Merged

Mixer: FunctionMotors: leave NAN control values at NAN with non-zero THR_MDL_FAC #24108

merged 2 commits into from
Dec 13, 2024
+9 −8

Conversation

sfuhrer
Copy link
Contributor

@sfuhrer sfuhrer commented Dec 13, 2024

Solved Problem

Fixes #24083.

Solution

Leave NAN untouched, even if THR_MOD_FAC is set.

Changelog Entry

For release notes: Mixer: leave NAN control values at NAN with non-zero THR_MDL_FAC to fix VTOL motors not turning off correctly.

Bugfix:

Alternatives

Should the whole THR_MOD_FAC logic not live in the control allocation? Why is it handled by the mixer?

Test coverage

SITL tested.

@sfuhrer sfuhrer added the bug label Dec 13, 2024
@sfuhrer sfuhrer requested a review from MaEtUgR December 13, 2024 15:39
@github-actions GitHub Actions
Copy link

🔎 FLASH Analysis

px4_fmu-v5x [Total VM Diff: 88 byte (0 %)] 
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%     +88  +0.0%     +88    .text
  +4.2%     +40  +4.2%     +40    ../../src/lib/mixer_module/actuator_test.cpp
  +0.5%     +32  +0.5%     +32    ../../src/lib/mixer_module/mixer_module.cpp
  +0.0%     +13  +0.0%     +13    [section .text]
  +0.2%      +3  +0.2%      +3    ../../src/systemcmds/ver/ver.cpp
+0.0%     +56  [ = ]       0    .debug_abbrev
   +11%     +56  [ = ]       0    ../../src/lib/version/version.c
-0.0%      -8  [ = ]       0    .debug_aranges
  -5.0%      -8  [ = ]       0    ../../src/lib/version/version.c
+0.0%    +104  [ = ]       0    .debug_info
  +0.4%     +67  [ = ]       0    ../../src/lib/mixer_module/actuator_test.cpp
  +0.1%     +41  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -0.2%      -4  [ = ]       0    ../../src/lib/version/version.c
+0.0%     +72  [ = ]       0    .debug_line
  +1.6%     +48  [ = ]       0    ../../src/lib/mixer_module/actuator_test.cpp
  +0.3%     +46  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  +0.3%      +3  [ = ]       0    task/task_cancelpt.c
+0.0%     +25  [ = ]       0    .debug_loc
  +6.9%    +144  [ = ]       0    ../../src/lib/mixer_module/actuator_test.cpp
  -0.6%    -101  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -0.0%     -18  [ = ]       0    [section .debug_loc]
-0.0%     -57  [ = ]       0    .debug_ranges
  -1.5%     -16  [ = ]       0    ../../src/lib/mixer_module/actuator_test.cpp
  -0.5%     -32  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -2.6%      -8  [ = ]       0    ../../src/lib/version/version.c
  -1.5%      -1  [ = ]       0    task/task_cancelpt.c
-0.5%     -88  [ = ]       0    [Unmapped]
+0.0%    +192  +0.0%     +88    TOTAL
px4_fmu-v6x [Total VM Diff: 72 byte (0 %)] 
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%     +72  +0.0%     +72    .text
  +4.2%     +40  +4.2%     +40    ../../src/lib/mixer_module/actuator_test.cpp
  +0.5%     +32  +0.5%     +32    ../../src/lib/mixer_module/mixer_module.cpp
+0.0%     +56  [ = ]       0    .debug_abbrev
   +11%     +56  [ = ]       0    ../../src/lib/version/version.c
-0.0%      -8  [ = ]       0    .debug_aranges
  -5.0%      -8  [ = ]       0    ../../src/lib/version/version.c
+0.0%    +104  [ = ]       0    .debug_info
  +0.4%     +67  [ = ]       0    ../../src/lib/mixer_module/actuator_test.cpp
  +0.1%     +41  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -0.2%      -4  [ = ]       0    ../../src/lib/version/version.c
+0.0%     +64  [ = ]       0    .debug_line
  +1.6%     +48  [ = ]       0    ../../src/lib/mixer_module/actuator_test.cpp
  +0.3%     +46  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -1.3%     -25  [ = ]       0    ../../src/lib/version/version.c
  -0.4%      -5  [ = ]       0    task/task_cancelpt.c
+0.0%     +41  [ = ]       0    .debug_loc
  +6.9%    +144  [ = ]       0    ../../src/lib/mixer_module/actuator_test.cpp
  -0.5%     -85  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -0.0%     -18  [ = ]       0    [section .debug_loc]
-0.0%     -57  [ = ]       0    .debug_ranges
  -1.5%     -16  [ = ]       0    ../../src/lib/mixer_module/actuator_test.cpp
  -0.5%     -32  [ = ]       0    ../../src/lib/mixer_module/mixer_module.cpp
  -2.6%      -8  [ = ]       0    ../../src/lib/version/version.c
  -1.5%      -1  [ = ]       0    task/task_cancelpt.c
-0.1%     -72  [ = ]       0    [Unmapped]
+0.0%    +200  +0.0%     +72    TOTAL

Updated: 2024-12-13T15:45:26

bresch
bresch approved these changes Dec 13, 2024
View reviewed changes
Copy link
Member

@bresch bresch left a comment

Choose a reason for hiding this comment

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

Good find

MaEtUgR
MaEtUgR approved these changes Dec 13, 2024
View reviewed changes
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this so quickly 👍
We should add unit tests for this and cover the cases that are part of the interface 👀

@MaEtUgR MaEtUgR merged commit 4db55cd into main Dec 13, 2024
60 checks passed
@MaEtUgR MaEtUgR deleted the pr-mixer-fix-thr-mdl-fac-main branch December 13, 2024 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bresch bresch bresch approved these changes

@MaEtUgR MaEtUgR MaEtUgR approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] THR_MDL_FAC causes VTOL motors not stop spinning during Fixed-wing cruising phase Firmware V 1.15
3 participants
@sfuhrer @MaEtUgR @bresch