Difference between revisions of "AirborneCodeReorg"
m |
|||
(29 intermediate revisions by 4 users not shown) | |||
Line 9: | Line 9: | ||
Fixed wing airframes have makefile directly in them - we can't change anything in the airborne code without breaking everybody's airframe | Fixed wing airframes have makefile directly in them - we can't change anything in the airborne code without breaking everybody's airframe | ||
--> use the subsystems | |||
There is also the "module" facility which is very good for optional features. | There is also the "module" facility which is very good for optional features. | ||
Line 33: | Line 16: | ||
*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 | *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 | ||
** | **fixedwing/main.c | ||
**thrust_vectoring/main.c | **thrust_vectoring/main.c | ||
**gps.[ch] | **gps.[ch] | ||
Line 43: | Line 26: | ||
==Coding Style== | ==Coding Style== | ||
* 2 spaces for indent | |||
* please no trailing whitespaces (e.g. fix with M-x whitespace-cleanup in emacs) | |||
== Proposals and RFC == | == Proposals and RFC == | ||
=== Directory structure proposals === | |||
relative to sw/airborne | relative to sw/airborne | ||
firmware/ | |||
rotorcraft/ | |||
rotorcraft.makefile | |||
rotorcraft.xml // list of subsystems: maybe autogenerate as this should reflect the directory structure | |||
foo/ | |||
foo.c | |||
foo_2.c | |||
fixedwing/ | |||
control/ | |||
control.c | |||
control_adaptive.c | |||
subsystems/ | |||
imu/ // a general subsystem with several implementations and arch depended code (initialization, interrupts, ...) | |||
imu_xsense.c | |||
imu_b2.c | |||
calibration_routines/ // might be useful to have subdirectories | |||
gps/ // another general subsystem | |||
.... | |||
radio_control/ // another general subsystem | |||
.... | |||
actuator | |||
stm - lpc - arm - omap - ... | |||
boards/ | |||
test_tiny | |||
test_lisa | |||
modules/ | modules/ | ||
.... | |||
arch/ | arch/ | ||
subsystems/ | |||
imu/ | |||
lcp21/ | |||
imu_xsense_arch.[ch] | |||
imu_b2_arch.[ch] | |||
stm32/ | |||
imu_xsense_arch.[ch] | |||
imu_b2_arch.[ch] | |||
sim/ | |||
firmware/ | |||
fixedwing/ | |||
control/ | |||
lcp21/ | |||
control_arch.[ch] | |||
control_adaptive_arch.[ch] | |||
stm32/ | |||
control_arch.[ch] | |||
control_adaptive_arch.[ch] | |||
sim/ | |||
each general susbsystem having several implementations gets a subdirectory under subsystems | |||
imu.[ch] | |||
imu. | |||
imu/ | imu/ | ||
imu_xsense.[ch] | imu_xsense.[ch] | ||
imu_b2.[ch] | imu_b2.[ch] | ||
Rationale for the subsystem directory: | |||
# to have less directories in the airborne root | |||
# to help auto-documentation tools: "all subfolders in airborne/subsystems/ are subsystems. if you put them directly in airborne it becomes: all directories in airborne are subsystems except arch/firmware/modules/... etc | |||
# it's less obvious how the imu code is used (is it standalone/firmware/modules) to newcommers than with the extra dir. | |||
==Test Code== | ==Test Code== | ||
Line 124: | Line 109: | ||
|- | |- | ||
| | | | ||
<target name=" | <firmware name=rotorcraft> | ||
<!-- this set of firmware fits in 2 targets: --> | |||
<target name="ap" board="booz"/> | |||
<target name="sim" board="sim" /> | |||
</target> | |||
<!-- these subsystems are used for all targets in this firmware --> | |||
<subsystem name="radio_control" type="ppm"/> | |||
<subsystem name="actuators" type="mkk"/> | |||
<subsystem name="imu" type="b2v1"/> | |||
</firmware > | |||
<firmware name="setup" > | |||
<target name="tunnel" /> | |||
<target name="test_actuators" /> | |||
</firmware> | |||
| | | | ||
include $( | ifeq ($(TARGET),ap) | ||
include $(PAPARAZZI_SRC)/conf/board/booz.makefile | |||
include $(PAPARAZZI_SRC)/conf/autopilot/rotorcraft.makefile | |||
include $(CFG_BOOZ)/subsystems/booz2_radio_control_ppm.makefile | include $(CFG_BOOZ)/subsystems/booz2_radio_control_ppm.makefile | ||
include $(CFG_BOOZ)/subsystems/booz2_actuators_mkk.makefile | include $(CFG_BOOZ)/subsystems/booz2_actuators_mkk.makefile | ||
include $(CFG_BOOZ)/subsystems/booz2_imu_b2v1.makefile | include $(CFG_BOOZ)/subsystems/booz2_imu_b2v1.makefile | ||
endif | |||
ifeq ($(TARGET),sim) | |||
include $(PAPARAZZI_SRC)/conf/board/pc.makefile | |||
include $(PAPARAZZI_SRC)/conf/autopilot/rotorcraft.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 | |||
endif | |||
ifeq ($(TARGET),tunnel) | |||
include $(PAPARAZZI_SRC)/conf/board/booz.makefile | |||
include $(PAPARAZZI_SRC)/conf/autopilot/setup.makefile | |||
endif | |||
|} | |} | ||
put defines in here that get written in airframe.h | ?? put defines in here that get written in airframe.h ?? | ||
[[User:Cdewagter|cdewagter]] I'm not really sure this is needed as otherwise you might put all other data of the airframe file in here too next to the submodule it belongs too. I think it is more readable with less defines in here. I would suggest to only have global compile defines that need to be in the Makefile.ac here... | |||
{| border=1 width=100% | {| border=1 width=100% | ||
| airframe.xml | | airframe.xml | ||
Line 145: | Line 159: | ||
|- | |- | ||
|rowspan=4| | |rowspan=4| | ||
<firmware name=fixedwing> | |||
<!-- this set of firmware fits in 2 targets: --> | |||
<target name="ap" board="tiny_2.11"> | |||
<define name="ALT_KALMAN"/> | |||
</target> | |||
</ | <subsystem name="led" /> | ||
</firmware > | |||
|- | |- | ||
| | | | ||
FLASH_MODE = IAP | |||
include $( | include $(PAPARAZZI_SRC)/conf/board/tiny_2.11.makefile | ||
include $( | include $(PAPARAZZI_SRC)/conf/autopilot/fixedwing.makefile | ||
ap.CFLAGS += -DALT_KALMAN | |||
include $(SRCS_FIXEDWING)/led.makefile | |||
|- | |- | ||
| airframe.h | | airframe.h | ||
|- | |- | ||
| | | | ||
#define USE_LED | |||
|} | |} | ||
Line 173: | Line 192: | ||
|- | |- | ||
| | | | ||
<subsystem name="gps" type="ubx"> | |||
<configure name="GPS_PORT" value="1"/> | |||
<configure name="GPS_BAUD" value="B38400"/> | |||
<configure name="GPS_LED" value="2"/> | |||
</subsystem> | |||
</ | |||
| | | | ||
GPS_PORT=1 | |||
GPS_BAUD= | GPS_BAUD=B38400 | ||
include $( | GPS_LED=2 | ||
include $(CFG_FIXEDWING)/subsystems/gps_ubx.makefile | |||
|} | |} | ||
* Note that now a subsystem can be enabled for all targets in that firmware by using | |||
$(TARGET).srcs += navigation.c | |||
* Note that you only 1 time in the board file (e.g. /boards/tiny_2.11.makefile) | |||
$(TARGET).arch = lpc21 | |||
instead having this in the firmware/subsystem makefiles | |||
tunnel.ARCH = lpc21 | |||
ap.ARCH = lpc21 | |||
fbw.ARCH = lpc21 | |||
... | |||
==Include guards== | ==Include guards== | ||
Line 257: | Line 291: | ||
* All headers are defined relative to sw/airborne (limit the -I to -Isw/airborne and the generated code) | * All headers are defined relative to sw/airborne (limit the -I to -Isw/airborne and the generated code) | ||
#include " | #include "foo/whatever.h" | ||
* Generated headers are indicated by their directory | * Generated headers are indicated by their directory | ||
#include "generated/airframe.h" | #include "generated/airframe.h" | ||
Line 268: | Line 302: | ||
I could not agree more! | I could not agree more! | ||
== things discuss or cleanup/remove == | |||
* remove sw/include/std.h? was for old avr stuff | |||
* cleanup TODO file | |||
* <strike>there is wind_tunnel under sw/airborne and under sw/in_progress</strike> | |||
* make a bin and/or build dir where all the ground sw build files and binaries go (paparazzicenter, gen_tools) to only have source files under sw | |||
[[Category:Software]] [[Category:Developer_Documentation]] |
Latest revision as of 10:16, 18 March 2013
Our Challenge
The current airborne sourcecode has gone a little bit out of control. Everyone was working on it enthusiastically, which is great! However, we feel now the moment has arrived where deep reorganization of the code would benefit us all.
It is really appreciated if you want to help out in this quest. Therefore remarks and suggestions can be placed on the page designed specifically for this. Of-course feel free to use the mailinglist also.
Problem areas
Fixed wing airframes have makefile directly in them - we can't change anything in the airborne code without breaking everybody's airframe
--> use the subsystems
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
- fixedwing/main.c
- thrust_vectoring/main.c
- gps.[ch]
- processor specific code (stm32, lpc21, avr, etc...)
- arch/lpc21/uart_arch.[ch]
- plateforme specific code (tiny, booz, lisa, etc...)
- blaaaaa
Coding Style
- 2 spaces for indent
- please no trailing whitespaces (e.g. fix with M-x whitespace-cleanup in emacs)
Proposals and RFC
Directory structure proposals
relative to sw/airborne
firmware/ rotorcraft/ rotorcraft.makefile rotorcraft.xml // list of subsystems: maybe autogenerate as this should reflect the directory structure foo/ foo.c foo_2.c fixedwing/ control/ control.c control_adaptive.c subsystems/ imu/ // a general subsystem with several implementations and arch depended code (initialization, interrupts, ...) imu_xsense.c imu_b2.c calibration_routines/ // might be useful to have subdirectories gps/ // another general subsystem .... radio_control/ // another general subsystem .... actuator stm - lpc - arm - omap - ... boards/ test_tiny test_lisa modules/ .... arch/ subsystems/ imu/ lcp21/ imu_xsense_arch.[ch] imu_b2_arch.[ch] stm32/ imu_xsense_arch.[ch] imu_b2_arch.[ch] sim/ firmware/ fixedwing/ control/ lcp21/ control_arch.[ch] control_adaptive_arch.[ch] stm32/ control_arch.[ch] control_adaptive_arch.[ch] sim/
each general susbsystem having several implementations gets a subdirectory under subsystems
imu.[ch] imu/ imu_xsense.[ch] imu_b2.[ch]
Rationale for the subsystem directory:
- to have less directories in the airborne root
- to help auto-documentation tools: "all subfolders in airborne/subsystems/ are subsystems. if you put them directly in airborne it becomes: all directories in airborne are subsystems except arch/firmware/modules/... etc
- it's less obvious how the imu code is used (is it standalone/firmware/modules) to newcommers than with the extra dir.
Test Code
Testing code is specifically for UAS of utmost importance. To test we could opt to make small cases in Testlink for everyone to try and report the outcome.
Replace makefile section with xml - [READY]
of course leave the possibility to have a makefile section, but the normal user would have something like
airframe.xml | => | Makefile.ac |
<firmware name=rotorcraft> <target name="ap" board="booz"/> <target name="sim" board="sim" /> <subsystem name="radio_control" type="ppm"/> <subsystem name="actuators" type="mkk"/> <subsystem name="imu" type="b2v1"/> </firmware > <firmware name="setup" > <target name="tunnel" /> <target name="test_actuators" /> </firmware> |
ifeq ($(TARGET),ap) include $(PAPARAZZI_SRC)/conf/board/booz.makefile include $(PAPARAZZI_SRC)/conf/autopilot/rotorcraft.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 endif ifeq ($(TARGET),sim) include $(PAPARAZZI_SRC)/conf/board/pc.makefile include $(PAPARAZZI_SRC)/conf/autopilot/rotorcraft.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 endif ifeq ($(TARGET),tunnel) include $(PAPARAZZI_SRC)/conf/board/booz.makefile include $(PAPARAZZI_SRC)/conf/autopilot/setup.makefile endif |
?? put defines in here that get written in airframe.h ??
cdewagter I'm not really sure this is needed as otherwise you might put all other data of the airframe file in here too next to the submodule it belongs too. I think it is more readable with less defines in here. I would suggest to only have global compile defines that need to be in the Makefile.ac here...
airframe.xml | => | Makefile.ac |
<firmware name=fixedwing> <target name="ap" board="tiny_2.11"> <define name="ALT_KALMAN"/> </target> <subsystem name="led" /> </firmware > | ||
FLASH_MODE = IAP include $(PAPARAZZI_SRC)/conf/board/tiny_2.11.makefile include $(PAPARAZZI_SRC)/conf/autopilot/fixedwing.makefile ap.CFLAGS += -DALT_KALMAN include $(SRCS_FIXEDWING)/led.makefile | ||
airframe.h | ||
#define USE_LED |
set makefile variables
airframe.xml | => | Makefile.ac |
<subsystem name="gps" type="ubx"> <configure name="GPS_PORT" value="1"/> <configure name="GPS_BAUD" value="B38400"/> <configure name="GPS_LED" value="2"/> </subsystem> |
GPS_PORT=1 GPS_BAUD=B38400 GPS_LED=2 include $(CFG_FIXEDWING)/subsystems/gps_ubx.makefile |
- Note that now a subsystem can be enabled for all targets in that firmware by using
$(TARGET).srcs += navigation.c
- Note that you only 1 time in the board file (e.g. /boards/tiny_2.11.makefile)
$(TARGET).arch = lpc21
instead having this in the firmware/subsystem makefiles
tunnel.ARCH = lpc21 ap.ARCH = lpc21 fbw.ARCH = lpc21 ...
Include guards
All headers have to be checked for missing include guards, style template:
#ifndef <MODULE>_<TARGET>_<FILENAME>_H #define <MODULE>_<TARGET>_<FILENAME>_H [code] #endif /* <MODULE>_<TARGET>_<FILENAME>_H */
Example for sw/airborne/csc/airspeed.h:
#ifndef CSC_AP_AIRSPEED_H #define CSC_AP_AIRSPEED_H [code] #endif /* CSC_AP_AIRSPEED_H */
Here is a list of headers know not to include guards please remove any that you update from the list.
sw/in_progress/videolizer/spook/conf_parse.h sw/in_progress/videolizer/spook/rtp.h sw/in_progress/videolizer/spook/stream.h sw/in_progress/videolizer/spook/log.h sw/in_progress/videolizer/spook/pmsg.h sw/in_progress/videolizer/spook/global_config.h sw/in_progress/videolizer/spook/outputs.h sw/in_progress/videolizer/spook/jpeg_tables.h sw/in_progress/videolizer/spook/frame.h sw/in_progress/videolizer/spook/inputs.h sw/in_progress/videolizer/spook/event.h sw/in_progress/videolizer/spook/mpegaudio.h sw/in_progress/videolizer/spook/encoders.h sw/in_progress/videolizer/spook/base64_table.h sw/in_progress/videolizer/spook/rtp_media.h sw/in_progress/videolizer/wis-go7007-linux/kernel/go7007-priv.h sw/in_progress/videolizer/ws-go7007-linux/kernel/wis-i2c.h sw/in_progress/videolizer/wis-go7007-linux/include/go7007.h sw/in_progress/inertial/C/tilt_display.h sw/airborne/fms/packet_header.h sw/airborne/arm7/interrupt_hw.h sw/airborne/arm7/lpcusb/usbhw_lpc.h sw/airborne/arm7/lpcusb/usbdebug.h sw/airborne/arm7/lpcusb/examples/console.h sw/airborne/arm7/lpcusb/examples/startup.h sw/airborne/arm7/lpcusb/examples/msc_bot.h sw/airborne/arm7/lpcusb/examples/spi.h sw/airborne/arm7/lpcusb/examples/msc_scsi.h sw/airborne/arm7/lpcusb/examples/blockdev.h sw/airborne/arm7/lpcusb/usbapi.h sw/airborne/arm7/test/bootloader/console.h sw/airborne/arm7/test/bootloader/usbhw_lpc.h sw/airborne/arm7/test/bootloader/startup.h sw/airborne/arm7/test/bootloader/usbdebug.h sw/airborne/arm7/test/bootloader/usbapi.h sw/airborne/arm7/test/bootloader/lpc21iap.h sw/airborne/arm7/test/welcome.h sw/airborne/csc/csc_vane.h sw/airborne/csc/csc_airspeed.h sw/airborne/vor/lo.h sw/airborne/coaxial/tl_gps_configure.h sw/airborne/flightzone.h sw/airborne/booz/arch/stm32/radio_control/booz_radio_control_ppm_arch.h sw/airborne/sd_card/sd_card.h sw/airborne/ism/uart_hw.h sw/airborne/sim/sim_uart.h sw/airborne/sim/ivy_transport.h sw/airborne/sim/led_hw.h sw/airborne/osam_imu_ugear.h sw/airborne/wind_tunnel/wt_baro.h
Misc
- All headers are defined relative to sw/airborne (limit the -I to -Isw/airborne and the generated code)
#include "foo/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!
things discuss or cleanup/remove
- remove sw/include/std.h? was for old avr stuff
- cleanup TODO file
there is wind_tunnel under sw/airborne and under sw/in_progress- make a bin and/or build dir where all the ground sw build files and binaries go (paparazzicenter, gen_tools) to only have source files under sw