Navigator: loiter: remove unnecessary _loiter_pos_set #21776
Merged
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
Signed-off-by: Silvan Fuhrer <silvan@auterion.com>
MaEtUgR
previously approved these changes
Jun 30, 2023
There was a problem hiding this comment.
It was introduced here: 8960ab3
But I cannot possibly understand what for. Maybe an unfinished thought about setting the loiter position once armed when already being in hold mode but I'm pretty sure it never worked correctly since as you mention the loiter point is never set again once hold mode is engaged 🤷♂️
Maybe a draft implementation contained a second call to set_loiter_position() and that's where it had an effect.
I also did some tests and I cannot find a case to enter the loiter's on_activation() with the flag true.
|
Idk why but the merge commit in main branch itself doesn't show these build failures, any clue why in this PR page itself it shows so many build errors? @MaEtUgR |
harrisondragoon
pushed a commit
to harrisondragoon/PX4-Autopilot
that referenced
this pull request
Jun 30, 2023
* Navigator: loiter: remove unnecessary _loiter_pos_set Signed-off-by: Silvan Fuhrer <silvan@auterion.com> * Navigator: loiter: remove commented line --------- Signed-off-by: Silvan Fuhrer <silvan@auterion.com> Co-authored-by: Matthias Grob <maetugr@gmail.com>
antbre
pushed a commit
to BioMorphic-Intelligence-Lab/PX4-Autopilot
that referenced
this pull request
Sep 14, 2023
* Navigator: loiter: remove unnecessary _loiter_pos_set Signed-off-by: Silvan Fuhrer <silvan@auterion.com> * Navigator: loiter: remove commented line --------- Signed-off-by: Silvan Fuhrer <silvan@auterion.com> Co-authored-by: Matthias Grob <maetugr@gmail.com>
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
While working on #21775 I noticed this seemingly unnecessary class variable
_loiter_pos_set, that was set to false inon_inactive()or inon_active, and then though only ever checked inon_activation(throughset_loiter_position()). I don't see how it could be false when entering on_activation(), and thus can just be removed, or am I missing something?Solution
Remove it!
Changelog Entry
For release notes:
Test coverage
Bit of SITL testing.