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 bindings for servo telemetry #28857

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

IamPete1
Copy link
Member

Adds bindings for servo telem, this allows servo checks to be done in lua.

@IamPete1
Copy link
Member Author

For refence attempt 1 cost 1500 bytes or so.
image

@IamPete1
Copy link
Member Author

IamPete1 commented Dec 21, 2024

This now gets the structure and adds auto generation of a bitmask check and set. This means the script writer does not need to know there is a validity bitmask in the structure. This is currently read only, but it also adds auto setting of the mask when a value is set. The only slight problem is that once set there is no way to un-set the mask from lua.

One disadvantage to this method is that were stuck with the types in the structure, previously I had returned the temp in degrees by applying the scale in the getter, now were stuck with centi degrees.

@IamPete1
Copy link
Member Author

Example of the new docs markup:

userdata AP_Servo_Telem::TelemetryData field command_position float'skip_check read valid_mask valid_types AP_Servo_Telem::TelemetryData::Types::COMMANDED_POSITION

First bit is the same, the stuff added is:

valid_mask valid_types AP_Servo_Telem::TelemetryData::Types::COMMANDED_POSITION

"valid_mask" is the keyword to enable the bitmask validity check.
"valid_types" is the variable name on the "AP_Servo_Telem::TelemetryData" object.
"AP_Servo_Telem::TelemetryData::Types::COMMANDED_POSITION" is the bitmask value that the mask should be checked against.

@IamPete1 IamPete1 requested a review from tpwrules December 21, 2024 04:05
@IamPete1
Copy link
Member Author

Now only costs about 1K, so saves 400 bytes over the individual getters.

image

@tpwrules
Copy link
Contributor

I haven't looked in detail, but it seems this can't absolutely fix the race condition because there isn't a lock protecting the telemetry data. The race window is much shorter now however, so maybe it's fixed enough? It was said in the call that there's servos which output different data at different rates anyway, so maybe comparing commanded vs. achieved just isn't possible in general.

Also still curious what the size impact would be if you had one mega function which returned eleven values.

Copy link
Contributor

@rmackay9 rmackay9 left a comment

Choose a reason for hiding this comment

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

Looks OK to me but @tpwrules will review more thoroughly

Copy link
Contributor

@tpwrules tpwrules left a comment

Choose a reason for hiding this comment

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

A couple little things in the generator but should be ready to go after!

I tested in SITL using quadplane-can and the position from four servos comes through but it's the same for all four of them. I checked that the data I could get from Lua matched the data logged in CSRV, and it all did so this PR is good, but there's still probably something funky here.

libraries/AP_Scripting/generator/src/main.c Outdated Show resolved Hide resolved
libraries/AP_Scripting/generator/src/main.c Outdated Show resolved Hide resolved
libraries/AP_Scripting/generator/src/main.c Outdated Show resolved Hide resolved
libraries/AP_Scripting/generator/src/main.c Outdated Show resolved Hide resolved
libraries/AP_Scripting/generator/src/main.c Outdated Show resolved Hide resolved
@IamPete1 IamPete1 force-pushed the servo_telem_scr branch 2 times, most recently from cbed113 to 19b0ebb Compare January 1, 2025 16:03
@IamPete1 IamPete1 requested a review from tpwrules January 1, 2025 16:07
IamPete1 and others added 2 commits January 4, 2025 22:31
Fix generator to skip generation of docs for generation methods that
don't exist, and to avoid generating Lua creation methods that couldn't
be called.

Co-authored-by: Thomas Watson <[email protected]>
@tpwrules tpwrules dismissed tridge’s stale review January 5, 2025 04:37

race condition (mostly) fixed

Copy link
Contributor

@tpwrules tpwrules left a comment

Choose a reason for hiding this comment

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

Tested and still works, thanks for the fixes.

I cleaned up the history slightly and folded in another really tiny fix to the generator. Figured it was easier to do it myself then to go through review again.

@tpwrules
Copy link
Contributor

tpwrules commented Jan 5, 2025

Concerned whether servo telemetry needs a fix similar to #28999 but that doesn't impact this PR at all.

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.

5 participants