Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

control: refactor rangeAidConditionsMet function #656

Merged
merged 2 commits into from
Oct 15, 2019

Conversation

bresch
Copy link
Member

@bresch bresch commented Oct 14, 2019

Just because it took me way too long to read this function.
After this refactor, the comment at the top of the function is almost redundant, should we remove it?

@bresch bresch added the non-functional Used for code changes that do not change functionality label Oct 14, 2019
@bresch bresch requested a review from RomanBapst October 14, 2019 11:48
@RomanBapst
Copy link
Contributor

@bresch Looks like a nice cleanup! Maybe you can rename "rangeAidConditionsMet" to "updateRangeAidEnabled" or "updateRangeAid" or something similar. The current name somehow makes me think that his function should return a boolean.

RomanBapst
RomanBapst previously approved these changes Oct 14, 2019
@RomanBapst
Copy link
Contributor

@bresch We need a test log where you can see the estimator switching in and out of range aid.
I usually use gazebo and add a a few boxes. Then I fly over them and verify that the drones does not change altitude.

rename rangeAidConditionsMet to checkRangeAidSuitability
@bresch bresch force-pushed the pr-range-aid-condition branch from 9199a9d to ef644c2 Compare October 14, 2019 12:33
@bresch
Copy link
Member Author

bresch commented Oct 14, 2019

@RomanBapst I changed the names and I did a sitl test with boxes:

https://logs.px4.io/plot_app?log=4bc72eb4-4d74-4aa2-a068-cae8828313e7
It switches properly in and out of range aid
2019-10-14_15-26-12_01_plot

@bresch
Copy link
Member Author

bresch commented Oct 15, 2019

@RomanBapst Can you review again please?

@bresch bresch merged commit e09e3e1 into master Oct 15, 2019
@bresch bresch deleted the pr-range-aid-condition branch October 15, 2019 09:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ekf non-functional Used for code changes that do not change functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants