AP_Scripting: Adds I2C transfer() bindings and an example #27287
Conversation
|
Sibling PR: #27288 |
IanBurwell
force-pushed
the
scr_transfer
branch
3 times, most recently
from
5b110f1 to
004b6b3
Compare
|
How do you feel about a string vs the tables? The pack and unpack string functions are very powerful for extracting values with the correct format. Its a shame we have the table precedent in the existing read function. |
|
I hadn't considered taking/returning a string which would be more memory efficient, and it would totally be satisfying to be able to do something like local some_uint32 = string.unpack("<I4", dev:transfer(string.pack("b", 0xAB), 4))where the string.unpack in handling the byte assembly into a uint32. But there is also a nice simplicity with using tables. If I wanted just one byte back after a multi byte write it seems like a bit much to do: local some_byte = string.unpack("b", dev:transfer(string.pack("bbb", 0xAB, 0xCD, 0xEF), 1))vs local some_byte = dev:transfer({0xAB, 0xCD, 0xEF}, 1)[1]But the more I think about this the more a string seems to help even if it incurs a bit more boilerplate. I am going to try and throw together something that uses strings instead. |
|
@IamPete1 I have added an alternative |
There was a problem hiding this comment.
I have only really reviewed the string version, I don't think we need both, it will just confuse people. Were tending to use strings now for serial and mavlink. I think string is the way to go, but then I have not used either. You have used both, which do you prefer?
|
Having used them both, I do now prefer the string version even though it brings some extra boilerplate with it. |
IanBurwell
force-pushed
the
scr_transfer
branch
2 times, most recently
from
89485d1 to
2b54710
Compare
|
I have added back the BQ40Z battery shutdown example as per discussion in the July 15th 2024 dev call and suggestion by @tridge. It works as desired on our dreamer quadrotors, and could be a useful example. With it this PR should be ready for merge pending review. |
| // map MAV_RESULT to a boolean | ||
| bool ok = result == MAV_RESULT_ACCEPTED; | ||
|
|
||
| lua_pushboolean(L, ok); | ||
| // Return the resulting MAV_RESULT | ||
| lua_pushinteger(L, result); |
There was a problem hiding this comment.
Be sure to update the docs, if not put this in a separate PR.
There was a problem hiding this comment.
Somehow that escaped me, thank you for the catch. I think it is not too out of scope of this PR and is required by it. I can split it into a different commit if you think that is justified.
There was a problem hiding this comment.
Yes, it definitely should be a separate commit.
I recognize it's necessary for the PR but it is technically backwards incompatible. It doesn't look like any examples currently check the return values though and it hasn't made it to a stable release so I'd be okay with it going in this PR.
This adds bindings for an I2CDevice's transfer() function, an example, and removes the nil return hint from the get_device() docs as it never actually returns nil.
This example intercepts PREFLIGHT_REBOOT_SHUTDOWN COMMAND_LONG's and if param1==2, it shuts down the BQ40Z smart battery BMS. Otherwise it passes through the COMMAND_LONG as a COMMAND_INT (this required updating the gcs:run_command_int to return a MAV_RESULT rather than a bool).
Adds LUA bindings for the I2CDevice
transfer()function. This is often needed overwrite_register()as I2C devices might not have 8 bit registers or desire some other custom transfer.An example script is also added that allows shutdown of a BQ40Z bms chip which would not be possible with LUA without the
transfer()function. This script has functionality similar to a sibling PR that implements such functionality more generally in the AP_BattMonitor domain.