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

AP_Arming: main loop slow triggers during motor test and compassmot #22388

Open
rmackay9 opened this issue Dec 13, 2022 · 11 comments · May be fixed by #22910
Open

AP_Arming: main loop slow triggers during motor test and compassmot #22388

rmackay9 opened this issue Dec 13, 2022 · 11 comments · May be fixed by #22910

Comments

@rmackay9
Copy link
Contributor

The arming check added by my PR #22320 is reportedly triggering after a motor test or compassmot. The issue is not so much the arming check itself but rather that these features stop the main loop which is probably bad behaviour.

We have two choices:

  1. add some kind of catch so that the arming check doesn't trigger during the use of these features
  2. modify these features to not stop the main loop

Reported by @andyp1per

@rishabsingh3003
Copy link
Contributor

I just reproduced this and was on my way to open this issue

@rmackay9
Copy link
Contributor Author

@rishabsingh3003,

Great, thanks for that. I have not been able to reproduce it so far. If you have any advice for reproducing it that would be great. Also you're sure of course that the autopilot won't trigger this pre-arm during a normal arm?

@prathamesh7255
Copy link
Contributor

@rmackay9 do you want it not to perform any arming check or do you want not to perform just the main loop slow check and perform all the remaining arming checks?

@rmackay9
Copy link
Contributor Author

rmackay9 commented Feb 2, 2023

@prathamesh7255,

As a minimum I think we can skip just this check if we are arming in order to do the motor test. I'm not sure though that I'm happy that we add an exception to the arming checks in order to do this.. I guess it depends upon how this is implemented.

Ideally I would like us to move these features so that they don't block the main loop. This is a bit more difficult though.

@prathamesh7255
Copy link
Contributor

@rmackay9 okay. If I am not asking a stupid question, when we say 'the main loop', may I know what loop we are talking about so that I can get an idea of what we can do?

@prathamesh7255
Copy link
Contributor

can we use asynchronous running so that the arming check and the main loop would run concurrently?
thank you.

@rmackay9
Copy link
Contributor Author

rmackay9 commented Feb 2, 2023

@prathamesh7255,

The "main loop" includes all the methods listed in the Copter.cpp's scheduler_tasks array. These tasks are listed in priority order and the scheduler (see AP_Scheduler's loop() method) executes as many as it can within 2.5ms (i.e. 400hz).

Yes, ideally we should make the motor test and compassmot asynchronous. It's slightly tricky to do though because you'd need to move any local variables within these two features to a structure within Copter.h. Then there would likely need to be a couple of new items added to that scheduler task list. The methods would need to be restructured so that they store their state and return within 1ms (or sooner) and then pickup where they left off when called again on the next iteration.

@prathamesh7255
Copy link
Contributor

@rmackay9 when you say methods woud need to be reconstructed, you mean AP_Scheduler's loop() method or anything else?

@prathamesh7255
Copy link
Contributor

it may take some time but i will try this

@rmackay9
Copy link
Contributor Author

rmackay9 commented Feb 6, 2023

Hi @prathamesh7255,

I meant that the compassmot and motor test methods will need to be restructured.

Good luck!

@prathamesh7255
Copy link
Contributor

Hi @prathamesh7255,

I meant that the compassmot and motor test methods will need to be restructured.

Good luck!

Okay, I have done my first job of moving all the local variables to a struct in the copter.h file.
thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants