I recently had time to deeply be involved in integrating PX4 into a project, and wanted to note couple of things that my colleague, along with my opinion, thought didn’t much make sense / was too hard in PX4, that probably prevents users from using it more easily.
For development, I think it is one of the best, because the structure is quite clear and modular and there are a lot of reference-able modules, drivers, etc.
- Having a : instead of ; in the class definition for a module results in errors such as “error: bit-field ‘bool CustomModule::init()’ has non-integral type ‘bool (CustomModule::)()’”. I wish it told that it’s a typo! (More of compiler issue)
- Having some typo results in “nuttx/config.h” not being able to be compiled, etc. Which is also an implicit compile error, and doesn’t point towards the actual typo / bug. This is due to the NuttX dependency.
There are so many checks. And ones like Airspeed, etc are easy to disable using a circuit breaker, but the tricky ones were:
- IMU consistency between two sensors
- GPS vertical velocity estimate accuracy
And in general the system is really ‘smart’ and prevents user from taking off in dangerous conditions, but also it isn’t so clear how those errors could be resolved unless diving into the code (which always ends up being more complicated than expected).
It does help a lot to have the event based failsafe / preflight failure reporting that directly exposes parameter editing tab (it’s awesome!) for changing appropriate parameters, but still I think overall it’s quite hard to figure out how to troubleshoot.
It was quite tricky to understand exactly how to disable certain gyro / IMU sensors. As it seemed that the PX4 was automatically recognizing the sensors and changing the parameters like detected gyro instance’s ID, Priority, Offset, etc.
So although we just wanted to “disable one IMU”, which probably should be a very easy thing to do, we had to change the Priority of the sensor to ‘disabled’, but then was faced with “No valid data in accel 0” and such.
This was quite daunting as I think the system should recognize that we don’t need to use 2 IMUs, and if one is disabled, it shouldn’t output errors and prevent arming because of that.
For cases like this, where would user be able to find info on “how to tell system that we are using only one IMU”? Shouldn’t disabling the sensor instance (via priority) be enough? (I think that’s already quite unintuitive)
To disable the mag, we just set the ‘priority’ of the mag to be 0, and then was faced with another error “You must set SYS_HAS_MAG to 0”.
I think perhaps this can be automatically detected, or these parameters should rather be exposed in a clean panel where user can configure which sensors to use, whether to disable them, how many of them there are, etc.
I think this is therefore maybe more of a QGC problem, of having more intuitive UI
We suddenly had errors “No valid global position available”, but didn’t really change anything in between that moment, and the test before that had EKF2 working properly.
It was really odd & hard to figure out what was actually wrong, and the fact that we can’t really pin-point where the problem is, because the codebase is so big was definitely a down side.
Maybe after more debugging, it would have been possible to figure out what the real error was, but EKF2 seems like a big beast that needs a lot of context to debug / use properly.
We figured out that the sensors onboard were different type than in the datasheet & saw that data coming from them were in wrong axis, etc.
And then I saw that we have the individual rotation enum setting for sensor instances (e.g. CAL_GYRO0_ROT), but it wasn’t really clear at what stage those rotations get applied.
After a while of search (going through PX4Gyroscope, Gyroscope, Sensor, individual drivers, I2C SPI bus class, etc), I was able to ‘suspect’ that this ROT parameter gets applied “after” the sensor_gyro data is published, and gets applied in the “sensor” module, then gets published in “vehicle_imu”.
But I wish this was documented somewhere in a better way, as it is really complicated. And I think that 99% of the users are always confused about things like: “Is sensor rotation supposed to be relative to the board FRD? vehicle body FRD?”.
I tried to document this better in PR like this:
However, still having that whole sensor pipeline documented and rotation enums would be great.
Alternatively, we should provide a nice UI in QGC to make sure users understand what “Sensor rotation” is based on (which frame of reference? board or the vehicle?)
Also having mechanism to just auto detect in a better way (right now everything is implicit and confusing to set rotation in QGC) would also be very helpful, along with graphical representation of the configurations shown.
Also, it was quite confusing as we could technically apply “rotaiton” to the internal sensors mounted on the PCB, and the correct way to do it is to set it to “internal” rotation, so that we assume that rc.board_sensors script has properly set the sensor rotation relative to the “board” frame (usually indicated by just an arrow on the PCB silkscreen). But anyhow, this was quite confusing!
The Flight modes page, especially the arm/killswitch/ etc page is broken (doesn’t allow setting arming switch, etc) for Ubuntu at least. Even testing the versions down to 4.2.0 still showed that the page was broken. This made it painful to change the ‘switch settings’ in the Parameters tab (which, for a newcommer would have been probably impossible)
Having a fast access (shortcut) to the MAVLink Shell, or having it “next to” the parameters edit panel would have been very helpful. Since in reality I would like to check the parameter value / names, while trying out commands in the MAVLink Shell.
Having a reboot button more exposed, instead of just in Parameters>Tools>reboot vehicle would have been great for developmental purposes. I have already proposed this as a PR: Add Reboot vehicle button in Main Window tool bar & Update Firmware update tab #10244 by junwoo091400 · Pull Request #10330 · mavlink/qgroundcontrol · GitHub
However, I must give credit to a lot of other parts that made development much better:
Logging system: The flexibility of adding custom topics is just unbeatable, and with PlotJuggler it is the best logging system I have used, and is essential for testing
Having pre-defined module base class definitions, etc: it made implementing a standalone thread running application a whiz, as there are .cmake templates already and well documented.
However, I did end up mistakingly writing those applications with a single loop (true for Work queue), which exited the program immediately for standalone thread applications
So some more documentation / warning on this would be great!