AirborneCodeReorg
Problem statement
Current airborne code has gone a little bit out of control and is in deep need of a reorganization.
Fixed wing airframes have makefile directly in them - we can't change anything in the airborne code without breaking everybody's airframe
ap.srcs += commands.c ap.CFLAGS += -DACTUATORS=\"servos_4015_MAT_hw.h\" -DSERVOS_4015_MAT ap.srcs += $(SRC_ARCH)/servos_4015_MAT_hw.c actuators.c ap.CFLAGS += -DRADIO_CONTROL ap.srcs += radio_control.c $(SRC_ARCH)/ppm_hw.c [...] ap.CFLAGS += -DNAV -DAGR_CLIMB -DLOITER_TRIM -DALT_KALMAN -DWIND_INFO ap.srcs += nav.c fw_h_ctl.c fw_v_ctl.c
Booz uses "susbsystem" Makefiles
include $(PAPARAZZI_SRC)/conf/autopilot/booz2_common.makefile include $(CFG_BOOZ)/booz2_autopilot.makefile include $(CFG_BOOZ)/subsystems/booz2_radio_control_ppm.makefile include $(CFG_BOOZ)/subsystems/booz2_actuators_mkk.makefile include $(CFG_BOOZ)/subsystems/booz2_imu_b2v1.makefile
This is the Makefile part of a booz vehicle using a booz2 IMU, mikrokopter actuators and ppm radio control
There is also the "module" facility which is very good for optional features.
There are 3 kinds of code in the airborne directory
- generic code (that doesn't depend on any architecture or plateforme) - there's a lot of it and it should probably be sorted by type of functionalities
- processor specific code (stm32, lpc21, avr, etc...)
- plateforme specific code (tiny, booz, lisa, etc...)
Coding Style
Names
Booz both stands for the LPC2148 quad board and the quadrotor autopilot code - Let's call the quadrotor autopilot "Baloo" ( if you got a better name, don't hesitate, but be quick). What about a name for the fixedwing code ? (which has been called Paparazzi until now and collides with the name of the project itself )
Proposed reorganization
- Move more code to modules
- Move Makefile out of the airframe file as much as possible (modules have there own makefile / booz include makefile approach: "conf/autopilot/fixedwing_common.makefile")
- Send mailing list message requesting to change the makefile section of airframes into new include structure
- Once many users have this in place and the wiki page is good, we can start to move code
Proposed Directory structure
relative to sw/airborne
baloo/ (a target) fixedwing/(or a new name, another target) radio_control/ (a subsystem) gps/ (another subsystem) ...(more subsystems) imu.[ch] imu/ modules/ arch/ avr/ lpc21/ stm32/ omap/
each susbsystem having several implementations gets a subdirectory
imu.[ch] imu/ imu_xsense.[ch] imu_b2.[ch]
in each of the directory under arch, a replication of the toplevel directory containing architecture specific implementations
imu.h imu/ imu_xsense.[ch] imu_b2.[ch] arch/ lpc21/ imu/ imu_xsense_arch.[ch] imu_b2_arch.[ch] stm32/ imu/ imu_xsense_arch.[ch] imu_b2_arch.[ch]
Test Code
Replace makefile section with xml
of course leave the possibility to have a makefile section, but the normal user would have something like
airframe.xml | => | Makefile.ac |
<target name="booz2_autopilot"> <subsystem name="radio_control" type="ppm"/> <subsystem name="actuators" type="mkk"/> <subsystem name="imu" type="b2v1"/> </target> |
include $(PAPARAZZI_SRC)/conf/autopilot/booz2_common.makefile include $(CFG_BOOZ)/booz2_autopilot.makefile include $(CFG_BOOZ)/subsystems/booz2_radio_control_ppm.makefile include $(CFG_BOOZ)/subsystems/booz2_actuators_mkk.makefile include $(CFG_BOOZ)/subsystems/booz2_imu_b2v1.makefile |
put defines in here that get written in airframe.h
airframe.xml | => | Makefile.ac |
<target name="booz2_autopilot"> <subsystem name="actuators" type="mkk"> <define name="NB" value="4"/> <define name="ADDR" value="{ 0x52, 0x54, 0x56, 0x58 }"/> </subsystem> <subsystem name="imu" type="b2v1"/> </target> | ||
include $(PAPARAZZI_SRC)/conf/autopilot/booz2_common.makefile include $(CFG_BOOZ)/subsystems/booz2_radio_control_ppm.makefile include $(CFG_BOOZ)/subsystems/booz2_imu_b2v1.makefile | ||
airframe.h | ||
#define ACTUATORS_MKK_NB 4 #define ACTUATORS_MKK_ADDR {0x52,0x54,0x56,0x58} |
set makefile variables
airframe.xml | => | Makefile.ac |
<target name="booz2_autopilot"> <subsystem name="gps" type="ubx"> <param name="GPS_UART_NR" value="1"/> <param name="GPS_BAUD" value="38400"/> </subsystem> </target> |
GPS_UART_NR=1 GPS_BAUD=38400 include $(CFG_BOOZ)/subsystems/booz2_gps_ubx.makefile |
Include guards
All headers have to be checked for missing include guards, style template:
#ifndef <MODULE>_<TARGET>_<FILENAME>_H
#define <MODULE>_<TARGET>_<FILENAME>_H
#endif /* <MODULE>_<TARGET>_<FILENAME>_H */
Example for sw/airborne/csc/airspeed.h:
#ifndef CSC_AP_AIRSPEED_H
#define CSC_AP_AIRSPEED_H
#endif /* CSC_AP_AIRSPEED_H */
Misc
- All headers are defined relative to sw/airborne (limit the -I to -Isw/airborne and the generated code)
#include "baloo/whatever.h"
- Generated headers are indicated by their directory
#include "generated/airframe.h"
Proposed methodology
[6:50:19 PM] Christophe De Wagter: "builing a new road is easy: nobody can ride until it is finished." ... "for road-works on an existing road there are 2 options: a) a non-important road can be closed for a while during roadworks b) but for roads that are too important: you need to keep a lane open: that is more work than closing it down but in the end keeps the total traffic problem minimal"
[6:51:05 PM] Christophe De Wagter: I think that a gradual approach might be slightly more work, but keeps the road open and developments running.
I could not agree more!
Let's start with a subsystem that is both simple enough yet representative, I suggest radio control
For now people using fixed wing have
ap.CFLAGS += -DRADIO_CONTROL
ap.srcs += radio_control.c $(SRC_ARCH)/ppm_hw.c
Let's ask people to remove those lines from their makefile and replace with
<target name="fixed_wings" board="tiny_2.11">
<subsystem name="radio_control" type="ppm">
</target>
which will generate the following in Makefile.ac
include $(PAPARAZZI_SRC)/conf/autopilot/fixed_wing_common.makefile
include $(PAPARAZZI_SRC)/conf/boards/tiny_2.11.makefile
include $(CFG_FIXED_WINGS)/radio_control_ppm.makefile
and the following in airframe.f
#define USE_RADIO_CONTROL 1