Skip to content

AP_Scripting: Only allow scripting restart if disarmed #24106

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

Closed

Conversation

IamPete1
Copy link
Member

@IamPete1 IamPete1 commented Jun 20, 2023

This requires the vehicle to be disarmed for scripting to be re-started. Scripts can still be stopped while armed. This is because I found another case where scripting restart causes a flight controller lockup and watchdog on real hardware.

In that case I restarted the math.lua test. There have been several of these that we have now fixed, but there might be more we have not found.

Crash dump output and watchdog decode from that one are here:
https://discord.com/channels/674039678562861068/719466647710072832/1120113508877598780

@timtuxworth
Copy link
Contributor

Scripting is being used and recommended for hard-core flight time work like device drivers. If it can stop and stay stopped, then those recommended uses might need to be re-considered.

@IamPete1
Copy link
Member Author

Scripting is being used and recommended for hard-core flight time work like device drivers. If it can stop and stay stopped, then those recommended uses might need to be re-considered.

Its not a script problem, its that restarting scripts can cause the flight controller to lock up. Its fine for the majority but there are some cases we have not tracked down.

If you don't try and restart there is no problem.

Copy link
Contributor

@tridge tridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to get to the bottom of the actual cause

@IamPete1
Copy link
Member Author

IamPete1 commented Jun 27, 2023

This is the reproduction:

  • Default params, SCR_ENABLE 1, SCR_VM_I_COUNT to 100000 (10x the default)
  • Load math.lua
  • Reboot and let it run, you should see Math tests passed messages
  • Restart scripting

I used a CubeOrange-Plus on master. The same thing works on stable, I have not tried other boards.

edit:

I have found a minimum script, just need this one line math.random(1, 0)

tridge added a commit to tridge/ardupilot that referenced this pull request Jul 4, 2023
@tridge
Copy link
Contributor

tridge commented Jul 4, 2023

@IamPete1 this PR fixes the issue for me:
#24218

tridge added a commit that referenced this pull request Jul 4, 2023
rmackay9 pushed a commit that referenced this pull request Jul 4, 2023
rmackay9 pushed a commit that referenced this pull request Jul 4, 2023
tridge added a commit that referenced this pull request Jul 4, 2023
@IamPete1
Copy link
Member Author

IamPete1 commented Jul 6, 2023

We have fixed the particular crash that prompted me to open this, #24231

However, I suspect there are others we have not found yet....

@IamPete1 IamPete1 closed this Jul 6, 2023
lgarciaos pushed a commit to lgarciaos/ardupilot that referenced this pull request Jul 10, 2023
117-MasterChief pushed a commit to 117-MasterChief/ardupilot that referenced this pull request Aug 5, 2023
AndersonRayner pushed a commit to aerorobotics/ardupilot that referenced this pull request Sep 11, 2023
peterbarker pushed a commit that referenced this pull request Jan 7, 2024
Dringl pushed a commit to dimaf405/ardupilot that referenced this pull request Apr 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants