Hi. As the current Toshiba RGB LED driver in the Pixhawk (V2) is end of life I have added support for a TI LED driver in the source code. When trying to find the files that had to be modified I found something strange. There are two files that both are compiled in and both have code for low level control of the Toshiba LED driver. These are /ardupilot/modules/PX4Firmware/src/drivers/rgbled/rgbled.cpp and /ardupilot/libraries/AP_Notify/ToshibaLED_I2C.cpp. At the end only modification to rgbled.cpp seem to have effect. I have the idea that this is also the case with controlling the buzzer. Some files are compiled but not actually used. Is there a reason for this? Is there a cleanup planned for obsolete source files?
ToshibaLED is a subclass of RGBLed and ToshibaLED_I2C is a subclass of ToshibaLED.
Also I don’t know why you think it is not used… It is used in VR Brain, Navio, Raspilot and for any other unidentified board.
Thanks for your reply. To be more clear I added the path to the files in my previous post. The files don’t seem to be related in any way, but are both controlling the LED (I2C traffic) and are compiled both when building for px4-v2 hardware. As a test: Swapping red and blue LED control in /ardupilot/modules/PX4Firmware/src/drivers/rgbled/rgbled.cpp has effect in hardware behavior. Swapping red and blue LED control in /ardupilot/libraries/AP_Notify/ToshibaLED_I2C.cpp has no effect at all. It’s not a real problem to me, but my experience learnt it’s not great having two pieces of code controlling the same hardware directly.
Sorry for taking so long to answer. The file in PX4Firmware is used in PX4 boards, the file in the libraries is used by Linux boards. They are both compiled because we compile each library as a whole but they aren’t both used (and even the final binary only has one of them).
Thanks for this. Compile time can be greatly reduced I guess.
Without any hard evidence, I would say that the time we would gain in compile time couldn’t beat the time we would lose maintaining the build system for that.