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.

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

Fix: Allow offboard acceleration control with only attitude and no horizontal velocity estimate #24076

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Dec 4, 2024

Solved Problem

When "ra1nyu" brought up https://discuss.px4.io/t/why-does-acceleration-control-in-offboard-mode-need-velocity-estimate/42512 in the developer call we found in the discussion that offboard pure acceleration control should not depend on a velocity estimate but only on vehicle attitude control.

Solution

Change requirement to pass check for acceleration setpoints when an attitude is available.

Changelog Entry

Fix: Allow offboard acceleration control with only attitude and no horizontal velocity estimate

Alternatives

I honestly did not go back in history to check why this dependency exists. I can still do that.

Test coverage

Not tested, I only made the pr while already looking at the code and discussing it.

@MaEtUgR MaEtUgR self-assigned this Dec 4, 2024
@DronecodeBot
Copy link

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/why-does-acceleration-control-in-offboard-mode-need-velocity-estimate/42512/7

Copy link

github-actions bot commented Dec 4, 2024

FLASH Analysis

px4_fmu-v5x
    FILE SIZE        VM SIZE    
--------------  -------------- 
[ = ]       0  [ = ]       0    TOTAL

px4_fmu-v6x
    FILE SIZE        VM SIZE    
--------------  -------------- 
+0.0%      +8  +0.0%      +8    .text
-0.0%      -8  [ = ]       0    [Unmapped]
[ = ]       0  +0.0%      +8    TOTAL

Updated: 2024-12-04T17:03:07

@beniaminopozzan
Copy link
Member

Hey @mrpollo , have the failing MAVROS tests been already listed down?

@mrpollo
Copy link
Contributor

mrpollo commented Dec 4, 2024

tests are fixed here #24039

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Dec 4, 2024

This was introduced either intentionally or by accident here:
d0c9a5f?diff=split#diff-e4b611e75eba3844661619867ee06befbcf230aced354a14f167f7d0ebbca8e0R3960

But then I guess @dagar would know and he didn't seem to have a particular case in mind.

@MaEtUgR MaEtUgR requested review from dagar and removed request for bkueng December 4, 2024 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants