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

Increase orbit radius limit & reject out of range radius commands #19362

Merged
merged 2 commits into from
Mar 28, 2022

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Mar 21, 2022

Describe problem solved by this pull request
For certain use cases the limit is just too low and if the limit is exceeded currently there a bug where the radius is just pruned instead of not executing the out of range orbit command.

Note: Not rejecting out of range radiuses is a regression:
9bd46be#diff-89127f427c1fc4e2fb02b97822ec75dc1bc7a4690ae4cd3aba8bc4f811250717L61

Describe your solution

  • the orbit radius is limited to 100m. If flying in open areas this limit is too strict.

    Makes it configurable by parameter. I'm open to suggestions for the default.

  • When commanding a larger radius in AMC, it gets resized to 100m.

    Fixes the bug by not acknowledging the command and stopping instead of executing the Orbit.
    This is already better than just changing the radius and then starting to execute but it should be further improved with e.g. an error message and possibly by switching mode.

Test data / coverage
SITL testing with out-of-range radiuses and hitting the limit by stick input.

potaito
potaito previously approved these changes Mar 21, 2022
Copy link
Contributor

@potaito potaito left a comment

Choose a reason for hiding this comment

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

LGTM

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Mar 21, 2022

😬

make: *** [Makefile:307: all_variants_px4_fmu-v2] Error 1
Memory region         Used Size  Region Size  %age Used
           flash:     1032465 B      1008 KB    100.03%
            sram:       27264 B       192 KB     13.87%
          ccsram:          0 GB        64 KB      0.00/opt/gcc/bin/../lib/gcc/arm-none-eabi/9.3.1/../../../../arm-none-eabi/bin/ld: px4_fmu-v2_test.elf section `.data' will not fit in region `flash'
/opt/gcc/bin/../lib/gcc/arm-none-eabi/9.3.1/../../../../arm-none-eabi/bin/ld: region `flash' overflowed by 273 bytes

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Mar 23, 2022

I had to rebase because of a conflict with https://github.com/PX4/PX4-Autopilot/pull/19367/files#diff-89127f427c1fc4e2fb02b97822ec75dc1bc7a4690ae4cd3aba8bc4f811250717R71-R72 but it's still the same just keeping signFromBool() and setting _started_clockwise when the command is accepted.

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Mar 28, 2022

Sorry but this also fails on master and I delayed this pr three times already because of issues on master 😇

@MaEtUgR MaEtUgR merged commit c7f114a into master Mar 28, 2022
@MaEtUgR MaEtUgR deleted the orbit-radius-limit branch March 28, 2022 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants