AP_Scripting: refresh serial bindings #27411
Conversation
Avoids the hassle of having to convert results to regular numbers in scripts. We trust the user will not be processing 2GiB of data in one call or attempting to go past 2 gigabaud.
Reduces likelihood of programming errors caused by forgetting to check the return value. Use a manual binding to ensure we return nil instead of nothing.
| // push the buffer as a string, truncated to the number of bytes actually read | ||
| luaL_pushresultsize(&b, read_bytes); | ||
| } else { | ||
| lua_pushnil(L); // error, return nil |
There was a problem hiding this comment.
You can just return 0 in this case. No need to push nil.
There was a problem hiding this comment.
Technically return 0 does not actually result in returning nil. It's not a big deal here but is more of a problem for port:read().
| if (port->read(c)) { | ||
| lua_pushinteger(L, c); | ||
| } else { | ||
| lua_pushnil(L); // error, return nil |
There was a problem hiding this comment.
We have to push nil specifically so that we return a value. Otherwise e.g. string.char(port:read()) will return an empty string if there is no data available instead of raising an error that it tried to do something with nil. (Previously, it would raise an error that -1 is out of range).
Avoids the pitfall of 0 being truthy in Lua. Now, `if port:write() then` will work as expected.
|
I do think this is a improvement, the key thing we have to decide is if were OK with the breaking change. |
|
Yeah, the breakage would be unfortunate. We could allow ourselves to remove uint32_t_ud return values by adding a metatable to numbers with the same methods. We also could deprecate the current serial It's questionable whether these are worth the flash space. They are not a high priority for me right now in any case. |
Removes some annoyances with unnecessary
uint32_t_udand pitfalls with -1 return values. This does technically break compatibility, not sure if there is any policy on that. I tried to edit the scripts in the repo to fix them but user scripts will need to be adjusted in minor ways.Tested in SITL and on Cube Orange with my REPL script.