PX4
WIP: New platform independent UART interface
Solved Problem
There are multiple copies of UART configuration and use code that are not platform independent. The Qurt platform, in particular, is quite different in terms of UART usage. This is a proposal for a new platform independent UART API that hides all of the implementation details in platform specific files and provides a generic API for all platforms. The GPS, RC, and UART based ESC drivers will important beneficiaries of this new API.
Its seems that on all non-Qurt targets the flash usage roughly increases by 2KB
Bloaty output px4-fmuv2
+0.2% +2.10Ki TOTAL
@dagar you might want to update bloaty on the CI since on many boards it gets an integer overflow.
PX4 pretty much assumes it's using POSIX based UART and uses termios to change settings. Wouldn't it be better to add a shim to platforms/qurt to implement the POSIX uart + termios for Qurt?
@dagar you might want to update bloaty on the CI since on many boards it gets an integer overflow.
Thanks @PetervdPerk-NXP, I'm going to update the containers to 22.04 + latest tools soon.
PX4 pretty much assumes it's using POSIX based UART and uses termios to change settings. Wouldn't it be better to add a shim to platforms/qurt to implement the POSIX uart + termios for Qurt?
That was one idea I proposed initially, but after some discussion with @katzfey and my own pain with subtle differences in POSIX between NuttX/Linux/MacOS I actually thought this could be a nice opportunity to simplify/optimize the vast majority of our serial usage with an API that's easier to use, harder to misuse.
- vast majority of serial usage is "raw" mode with that wall of termios + cfsetispeed configuration duplicated across most drivers
- we don't actually use most of POSIX terminal I/O past initial config where we disable everything to get to raw mode
- there are subtle differences between NuttX/Linux/MacOS with O_NONBLOCK, cfsetspeed, VTIME, VMIN, etc
- opportunity to optimize the typical usage pattern where we want at least a certain number of bytes with minimal latency (NuttX doesn't actually implement VMIN/VTIME)
- simpler setup and cleanup (RAII, no direct file descriptors, etc)
- possibly a bit easier for future ports to Zephyr or even crazier ideas like a native windows build (not a major consideration)
The immediate goal in this PR was to get to an acceptable MVP to unblock QuRT progress and leave room to continue some of the other ideas in the future.
Happy to discuss further before we fully commit to anything.
I just come up with some other functionality requirements related to UART:
- Single-Wire mode
- Inverted mode
- WorkQueue integration? (mostly about
px4::serial_port_to_wq()so that per-bus thread/task can be achieved) - Possibility to do non-blocking IO with
FIONREAD&FIONSPACEsupport instead of fire-until-timeout? (not sure if any driver requires that)
Just gave up reading the mavlink module for its size XD Not sure what requirement is needed for those complex, software throttling enabled modules.
The temporary increased code size will be reduced once drivers switching to new API for sure... Just noticed so many redundant occurrences...
@SalimTerryLi The idea with this PR is to get started with GPS only. So the new API has enough functionality for only what GPS needs and GPS is updated to use the new API. This is actually a pretty standard configuration so it should be usable by most other drivers. But as more drivers get updated to use the new API any missing additional functionality or settings that they need will need to be added to the Serial driver at that time. This will especially be true for the RC drivers as they have a bunch of oddball requirements.
Oh this went in? I'm still missing the routines for TX/RX swap, TX/RX invert and Single-wire mode. Any roadmap to get this in? Kinda sucks to have 2 living UART subsystems.
@PetervdPerk-NXP Yes, those will be coming in as well. The idea was to add enough functionality for the drivers being ported over. Initially this is just GPS where those other features are not used. But today I am working on PR 22917 which requires flush, Tx / Rx swap, and single wire. So those will be added in that PR. Next on the list is UART distance sensor and RC Library which should bring in all the rest of the needed UART functions.
This unfortunately breaks Septentrino GPS with the warnings:
ERROR [SerialImpl] readAtLeast: Buffer not big enough to hold desired amount of read data
Hi,Eric. I wanna rebuild my code with your serial's API. I follow what you did over the gps's driver. However it encounters error while it compiles. What do I miss?
The error:/Users/kevinchen/PX4-Autopilot/src/drivers/gasengine/gasengine.hpp:139:2: error: 'Serial' does not name a type
139 | Serial _uart {}; ///< UART interface to gaseng
| ^~~~~~
compilation terminated due to -Wfatal-errors.
@kevinuav Did you "#include <px4_platform_common/Serial.hpp>"? Solved. I forgot the namespace. Sorry. And thank you.
Hi, Katzfey. I find a bug in you PX4's Serial API.I use the readAtLeast() to get the command reply from my uart device. The device will reply my command first. Then it sends back the messages accord to my command. Just like this: I send: 1,APE,1\n It sendback:1,APE,1\n message....... However I find the readAtLeast() can't send back the command"1,APE,1\n". It directly outputs the messages follow behind. Why? Is it designed to cut off the first characters I just sent? Or it is just a bug?
@kevinuav Are you saying it works as expected when you don't use the Serial API, but when you switch to the Serial API you lose part of the response? What platform are you on? NuttX?
@kevinuav Are you saying it works as expected when you don't use the Serial API, but when you switch to the Serial API you lose part of the response? What platform are you on? NuttX?
I have just checkout the latest firmware.It should be the latest version of NuttX. I am using CUAV's V5-nano. The compile target is px4_fmu-v5_default. Does the function readAtLeast() reads the datas from one"\n" to another "\n"? If it is true. Then it will ignore the first command since it ends with "\n". That makes sense.
The code for nuttx readAtLeast is here: https://github.com/PX4/PX4-Autopilot/blob/36d89df0a722e2fca3f23e4d0ddd9da9fb5b7ecc/platforms/nuttx/src/px4/common/SerialImpl.cpp#L254. You can see that it does not look at the data. What code are you working with that is using the Serial API?
The code for nuttx readAtLeast is here:
https://github.com/PX4/PX4-Autopilot/blob/36d89df0a722e2fca3f23e4d0ddd9da9fb5b7ecc/platforms/nuttx/src/px4/common/SerialImpl.cpp#L254
. You can see that it does not look at the data. What code are you working with that is using the Serial API?
Hi,Katzfey. I am sorry. After countless testing. I find that It is not your Serial API's issue. It is the bug of the sprint() function. It will end up with '00' not the needed '\r' . Then it will trigger the "no command reply" mode.of the device. So using the Serial API replied message are difference from using the computer's serial assister. I am sorry.