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_Scripting: Support REPL over MAVLink #13547

Merged
merged 3 commits into from
Feb 19, 2020

Conversation

WickedShell
Copy link
Contributor

@WickedShell WickedShell commented Feb 11, 2020

This allows a GCS to have a REPL (Read Eval Print Loop) running on it. This allows a user to type in some experimental code in the GCS, submit it, and have the autopilot run and print the result out back to the GCS. This enables you to test code snippets on the fly, or manipulate subsytems experimentally before saving the useful bits to a script. This is both very valubale while developing scripts, it actually starts to enable a replacement for the NSH shell, as you can start to query and manipulate our libraries on demand.

At the moment starting a REPL session completely blocks all lua scripts from running until it's completed the session. This is a restriction which could be removed in the future, but at the moment I'm quite comfortable accepting that as a restriction, as in the common use case you don't need to mux both scripts and REPL at the same time.

This leverages MAVFTP as our underlying transport mechanism to get data to and from the autopilot. a GCS reads/writes to a pair of files located in the repl (or /APM/repl on a chibios board) folder. It appends new commands to the input file, and reads the new output from the back of the output file. It's a bit messy that there are 2 files for this, ideally this would move to a pair of virtual files which didn't get bounced to disk, but that will need quite a lot more memory, and a lot more effort to get to.

This also adds a new MAV_CMD for scripting, which is currently used to both start and stop REPL sessions. It should be easy to add a new option to this command to restart all currently running scripts, which is a often requested feature.

This seems to be doing well in my tests so far, but I'd like to see a couple of other GCS authors create an actual client before we bring this in, I tested it by just FTP'ing new files up and down to the card directly, without a fancy GUI client that looked like a more normal console.

As an obvious limitation this is pointing at a commit I pushed into the MAVLink submodule that needs to come in before we merge this.

This has been tested on both a CubeBlack (well green) and SITL so far.

@@ -100,8 +100,57 @@ void AP_Scripting::init(void) {
}
}

MAV_RESULT AP_Scripting::handle_command_int_packet(const mavlink_command_int_t &packet) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: normally brackets are on the following line.. obviously not a blocker just mentioning it

@tridge
Copy link
Contributor

tridge commented Feb 17, 2020

rebase and valgrind test with changes, thanks!

@WickedShell
Copy link
Contributor Author

Rebased, no problems with valgrind, found a couple of small errors.

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.

None yet

4 participants