AP_Scripting: added external AHRS scripting backend

[tridge]

This allows lua scripts to provide the following backends:

• external AHRS (position, velocity, attitude etc)
• GPS
• Baro
• Compass
• airspeed
• IMU

It builds on top of #25674 and re-implements the InertialLabs driver as a lua script
also builds on:

• AP_Scripting: added SCR_THD_PRIORITY #25686
• AP_Scripting: added readstring for uarts #25685

[IamPete1]

Why do we need the drivers for the anything other than external AHRS? Currently the external AHRS lib already populates all the libraries directly with the existing drivers? Why does the scripting driver need to publish via external AHRS and directly to the sensor driver?

[tridge]

Currently the external AHRS lib already populates all the libraries directly with the existing drivers?

no, it doesn't, it is the individual backends that do that, not the library itself. It really needs to be this way as the backend knows what makes sense to publish and can have more details that the frontend knows (eg. backend usually knows the GPS details, frontend only knows the estimator solution)

[IamPete1]

no, it doesn't, it is the individual backends that do that, not the library itself. It really needs to be this way as the backend knows what makes sense to publish and can have more details that the frontend knows (eg. backend usually knows the GPS details, frontend only knows the estimator solution)

Currently this tells external AHRS the accel and gyro and then passes the same information to the INS lib. Why not just pass the information to AHRS and have it pass on to the INS lib rather than doing it in the script? There is not the duplication for the other sensors, but you could include them in the state, or have new function that passes though to the sensor lib.

My concern is more really about the code structure and the data flow. Its useful for all the data to flow the same way between all the different back-ends. It makes testing and debugging easier if there are few "entry points" for the data from the script. I guess the counter argument is that the script itself is a external AHRS back end so it is the same as the others. I think I would tend to make the distinction at the scripting bindings, the script itself is part of the external AHRS unit not part of AP. Some use UART to get the data into AP, the scripting back end used the scripting bindings.

[tridge]

Why not just pass the information to AHRS and have it pass on to the INS lib rather than doing it in the script?

this is by design, and is quite important.
When the external AHRS is active we use the external AHRS gyro and accel for the AHRS data products such as the EF accel used for landing detection. This makes it all consistent with the velocity data we are providing.
Meanwhile, the external AHRS can also (optionally!) be presenting its IMU data to AP_InertialSensor. If enabled (using EAHRS_SENSORS) then this makes the IMU data available to other estimators such as EKF3 or DCM. If we only provided it via AHRS then it would not be possible to run EKF3 with the external IMU, and that is one of the more useful cases if you have something like a FOG which has extremely high quality IMUs, but don't fully trust the black box state estimation inside the FOG (with its lack of good logging, no replay etc)
The same applies for baro, gps, mag, airspeed. In each case there is a distinct difference between the external system providing a serial connected sensor and it providing a data product for use by AHRS.

[IamPete1]

This confused me for a few moments, that extra _ is a bit subtle. Maybe new_state rather than _state?

[IamPete1]

It might be worth a rename on this, its the creation function so you would have to rename the whole object.

Its a little late now, but I wonder it we would have been better to namespace these userdata. Currently you do:

data = mag_data_message_t()

We could name space it so the creation is not global:

data = compass:mag_data_message_t()

[IamPete1]

The generator gives you a default description all of the user data fields "set field" or "get field". So I think have a field called "field" is confusing. Maybe the object could be compass_data_message and the filed mag_field.

[IamPete1]

Why not a vector 3? Or a vector 2 for north/east and a float for down?

[IamPete1]

This is quite verbose, some of the others are really not. "vdop", "ms_tow".

[IamPete1]

Adding the semaphore at this point mean that all the methods call it. Why not just add it in the cpp for the handle_external function? Like wise for all the others.

[IamPete1]

I'm amazed airspeed is not used a local variable in any of our example scripts. But this might conflict with scripts in the wild.

I think its OK because the local variable with overwrite the global so the script will work, just not the this new lib, but it might be worth checking, would also be interesting to see if luacheck would catch it.