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

Commander class structure instead of scattered static states #13612

Closed
MaEtUgR opened this issue Nov 27, 2019 · 3 comments
Closed

Commander class structure instead of scattered static states #13612

MaEtUgR opened this issue Nov 27, 2019 · 3 comments

Comments

@MaEtUgR
Copy link
Member

MaEtUgR commented Nov 27, 2019

Describe problem solved by the proposed feature
I'm trying to split out all the preflight/arming logic of the commander into modular classes with the main goal to have them more understandable and callable in a modular way on a regular basis. #13224 was the first step of this but now I'm running against a wall adding member variables to the classes and having the class instances be part of the commander module class since there are so many calls from outside static functions.

One chunk of Commander improvements also listed in #7055 is to have the Commander module as a class holding all the split out subfunctions instead of having all the remaining static states and functions e.g. here https://github.com/PX4/Firmware/blob/214e9c8244bfca72a319694ae222727f658ca6c1/src/modules/commander/Commander.cpp#L112-L270

Describe your preferred solution
I think a really big point that is missing is getting rid of the low priority thread for calibration and a solution moving it to the events was last tracked here: #8706 (older PRs for the same thing #8200 #8630) and after that I think it's a lot easier to achieve this goal step by step because the low priority thread is in my eyes the blocker for refactoring since it's more complicated from a scheduling perspective.

I plan on trying to take this up again and do progress. Can we collect the main helping pointers and concerns here such that it gets easy to take a stab?

FYI @dagar

@MaEtUgR
Copy link
Member Author

MaEtUgR commented Nov 28, 2019

Update: @dagar Already opened a pr yesterday to work into that direction: #13613 (Thanks for that!)

@stale stale bot added the stale label Feb 27, 2020
@PX4 PX4 deleted a comment from stale bot Sep 6, 2020
@stale stale bot removed the stale label Sep 6, 2020
@stale
Copy link

stale bot commented Dec 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale label Dec 25, 2020
@bkueng
Copy link
Member

bkueng commented Oct 25, 2022

All done

@bkueng bkueng closed this as completed Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants