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: Add generator #10866

Merged

Conversation

WickedShell
Copy link
Contributor

This adds the generator demo from the unconference. It adds a binding generator, suppresses the console prints by default, and fixes the variable shadowing that was breaking compilation.

It also adds a singleton to AP_Notify and promotes play_string to be a public method to let scripts play tones.

@WickedShell
Copy link
Contributor Author

Removed some extra libraries from the sandbox/default load which saves 24k flash, and runtime ram

@magicrub
Copy link
Contributor

magicrub commented Apr 8, 2019

needs rebase

@peterbarker
Copy link
Contributor

Michael and I have discussed the Notify changes.

These allow scripts to play tunes, which is a great feature.

I have reservations about having play_tune on AP_Notify (we don't have a flash_leds interface at the moment, for example - but perhaps we should?). Perhaps it would be better to have a hal.play_tune(const char *str);.

Additionally, with the Notify changes here the handling of the play_tune mavlink message should move out of the backend.

Neither of these are showstoppers and should not hold up merging here.

@WickedShell
Copy link
Contributor Author

Following the dev call conversation I'm going to go ahead and bring this in now. I've been unable to get synced with Tridge during the week. The conclusion on the call was that it was fine to bring this in as long as it doesn't have an impact while disabled. Given the review on the notify changes from peter I'm going to pull this in now.

@WickedShell WickedShell merged commit f1d5269 into ArduPilot:master Apr 12, 2019
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

3 participants