Commander: remove COM_POS_FS_DELAY
#24063
Merged
+2
−18
Conversation
This file contains 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
A user configurable delay for the internal `vehicle_local_position` seems confusing in my eyes. It's a different timeout for fixed-wing and multirotor which might have made sense earlier but not really anymore since the topic is constantly published by the estimator and not expected to time out on either vehicle type and the parameter description is also misleading because it's outdated.
FLASH Analysispx4_fmu-v5xpx4_fmu-v6xUpdated: 2024-12-02T15:22:12 |
sfuhrer
approved these changes
Dec 2, 2024
bresch
approved these changes
Dec 3, 2024
There was a problem hiding this comment.
This sets number of seconds that the position checks need to be failed before the failsafe will activate
Looks like the original implementation was a failsafe reaction delay, not just a timeout. 1s timeout on the local position message is reasonable and does not need to depend on the vehicle type (they all run the same estimator with same data rates).
|
@MaEtUgR This is referenced in https://docs.px4.io/main/en/config/safety.html#position-gnss-loss-failsafe - what change is needed? Is it simply that after 1s of not having a position estimate the failsafe triggers? Or to put it another way, does this capture what is needed in the docs PX4/PX4-user_guide#3479 |
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.
Solved Problem
When answering a support question I found that currently
COM_POS_FS_DELAYconfigures the timeout for the internalvehicle_local_positionuORB message which is to my knowledge not expected to time out as long as the estimator is running since it gets published regularly also when the position (or altitude) is flagged invalid.A user-configurable timeout for the internal
vehicle_local_positionseems confusing in my eyes. It's a different timeout for fixed-wing and multirotor which might have made sense earlier but not really anymore since the topic is constantly published by the estimator and not expected to time out on either vehicle type and the parameter description is also misleading because it's outdated.Solution
I remove the parameter and hardcode the timeout for the
vehicle_local_positionto 1 second.Changelog Entry
Alternatives
Is the estimate published so much less often when dead-reckoning as fixed-wing? If yes, shouldn't that fact then not be represented within the uORB message instead of a user-facing parameter?
Test coverage
No particular tests done.
Context
The end-user was looking to delay the failsafe land when the local position gets invalid and found a match with the parameter description but it didn't work as expected.