directory structure for architecture-specific code and headers #12821

bkueng · 2019-08-28T10:06:18Z

PR from #12797. This includes everything except for the last 3 commits (the 'include' directory change, we can do that separately).
Changes from #12797: determine NuttX chip from NuttX config instead of specifying it in the board config.

Chip-specific directory structure:

platforms/nuttx/src/px4> tree
.
├── CMakeLists.txt
├── common
│   ├── CMakeLists.txt
│   ├── console_buffer.cpp
│   ├── include
│   │   └── px4_platform
│   │       ├── adc.h
│   │       └── micro_hal.h
│   ├── px4_init.cpp
│   ├── px4_nuttx_impl.cpp
│   └── px4_nuttx_tasks.c
├── nxp
│   ├── CMakeLists.txt
│   ├── k66
│   │   ├── CMakeLists.txt
│   │   └── include
│   │       └── px4_arch
│   │           ├── adc.h
│   │           ├── io_timer.h
│   │           └── micro_hal.h
│   └── kinetis
│       ├── adc
│       │   ├── adc.cpp
│       │   └── CMakeLists.txt
│       ├── hrt
│       │   ├── CMakeLists.txt
│       │   └── hrt.c
│       ├── include
│       │   └── px4_arch
│       │       ├── adc.h
│       │       └── io_timer.h
│       ├── io_pins
│       │   ├── CMakeLists.txt
│       │   ├── input_capture.c
│       │   ├── io_timer.c
│       │   ├── pwm_servo.c
│       │   └── pwm_trigger.c
│       ├── led_pwm
│       │   ├── CMakeLists.txt
│       │   └── led_pwm.cpp
│       └── tone_alarm
│           ├── CMakeLists.txt
│           └── ToneAlarmInterface.cpp
└── stm
    ├── CMakeLists.txt
    ├── stm32_common
    │   ├── adc
    │   │   ├── adc.cpp
    │   │   └── CMakeLists.txt
    │   ├── hrt
    │   │   ├── CMakeLists.txt
    │   │   └── hrt.c
    │   ├── include
    │   │   └── px4_arch
    │   │       ├── adc.h
    │   │       ├── io_timer.h
    │   │       ├── micro_hal.h
    │   │       └── px4io_serial.h
    │   ├── io_pins
    │   │   ├── CMakeLists.txt
    │   │   ├── input_capture.c
    │   │   ├── io_timer.c
    │   │   ├── pwm_servo.c
    │   │   └── pwm_trigger.c
    │   ├── led_pwm
    │   │   ├── CMakeLists.txt
    │   │   └── led_pwm.cpp
    │   └── tone_alarm
    │       ├── CMakeLists.txt
    │       ├── ToneAlarmInterface.cpp
    │       ├── ToneAlarmInterfaceGPIO.cpp
    │       └── ToneAlarmInterfacePWM.cpp
    ├── stm32f1
    │   ├── CMakeLists.txt
    │   ├── CMakeLists.txt.orig
    │   ├── include
    │   │   └── px4_arch
    │   │       ├── io_timer.h
    │   │       └── micro_hal.h
    │   └── io_timer.h
    ├── stm32f4
    │   ├── CMakeLists.txt
    │   ├── include
    │   │   └── px4_arch
    │   │       ├── adc.h
    │   │       ├── io_timer.h
    │   │       ├── micro_hal.h
    │   │       └── px4io_serial.h
    │   └── px4io_serial
    │       ├── CMakeLists.txt
    │       └── px4io_serial.cpp
    └── stm32f7
        ├── CMakeLists.txt
        ├── include
        │   └── px4_arch
        │       ├── adc.h
        │       ├── io_timer.h
        │       ├── micro_hal.h
        │       └── px4io_serial.h
        └── px4io_serial
            ├── CMakeLists.txt
            └── px4io_serial.cpp

updated: tone_alarm, px4io_serial, px4_micro_hal

It belongs there since it's not a library providing the implementation, but just declaring the interface.

…rivers/driver_framework_wrapper

Script to update include paths: for i in $(grep -rl 'include <px4_work_queue' src platforms); do sed -i 's/#include <px4_work_queue/#include <px4_platform_common\/px4_work_queue/' $i; done

delete include: for i in $(grep -rl 'px4_middleware.h' src platforms); do sed -i '/#include <px4_middleware.h/d' $i; done

bkueng · 2019-08-29T05:56:13Z

The output of bloaty looks as expected:

2019-08-28 13:18:38 URL:https://s3.amazonaws.com/px4-travis/Firmware/master/px4_fmu-v2_default.elf [14036408/14036408] -> "master.elf" [1]

     VM SIZE    

 -------------- 

  [NEW]    +352    ArchPX4IOSerial::_bus_exchange()

  [NEW]    +232    ArchPX4IOSerial::ioctl()

  [NEW]    +188    ArchPX4IOSerial::init()

  [NEW]    +168    px4_arch_adc_init

  [NEW]    +154    ArchPX4IOSerial::~ArchPX4IOSerial()

  [NEW]    +120    ArchPX4IOSerial::_do_interrupt()

  [NEW]     +94    px4_arch_adc_sample

  [NEW]     +68    ArchPX4IOSerial::_io_buffer_storage

  [NEW]     +52    ArchPX4IOSerial::ArchPX4IOSerial()

  [NEW]     +52    ArchPX4IOSerial::_abort_dma()

  [NEW]     +52    ArchPX4IOSerial::_do_rx_dma_callback()

  [NEW]     +40    ArchPX4IOSerial

  [NEW]     +32    serial_dma_call.9386

  +0.0%     +25    [section .text]

  +6.2%     +24    [section .bss]

  [NEW]     +20    channels_cache.9383

  [NEW]     +14    ArchPX4IOSerial::_interrupt()

  [NEW]     +10    ArchPX4IOSerial::_dma_callback()

  +8.5%      +8    ADC::~ADC()

  [NEW]      +8    base_time.9275

  [NEW]      +6    px4_arch_adc_temp_sensor_mask

  [NEW]      +4    boards_manifest.8842

  [NEW]      +4    k.9407

  [NEW]      +4    last_count.9276

  [NEW]      +4    px4_arch_adc_init::once

  [NEW]      +2    px4_arch_adc_uninit

  [ = ]       0    Airspeed::probe()

  [ = ]       0    LedController::update()

  [ = ]       0    PX4IO_Uploader::~PX4IO_Uploader()

  [ = ]       0    PX4IO_serial::init()

  [ = ]       0    [Unmapped]

  [ = ]       0    [section .debug_abbrev]

  [ = ]       0    [section .debug_aranges]

  [ = ]       0    [section .debug_frame]

  [ = ]       0    [section .debug_info]

  [ = ]       0    [section .debug_line]

  [ = ]       0    [section .debug_loc]

  [ = ]       0    [section .debug_ranges]

  [ = ]       0    [section .debug_str]

  [ = ]       0    [section .strtab]

  [ = ]       0    arm_disarm()

  [ = ]       0    cdev::CDev::poll_state()

  [ = ]       0    up_pwm_servo_init

  [DEL]      -4    board_adc_init::once

  [DEL]      -4    boards_manifest.8847

  [DEL]      -4    k.9412

  [DEL]      -4    last_count.9281

  [DEL]      -8    base_time.9280

  [DEL]     -10    PX4IO_serial_f4::_dma_callback()

  [DEL]     -14    PX4IO_serial_f4::_interrupt()

  -0.0%     -17    g_cromfs_image

  [DEL]     -20    channels_cache.9388

  [DEL]     -32    serial_dma_call.9391

  [DEL]     -40    PX4IO_serial_f4

  [DEL]     -52    PX4IO_serial_f4::PX4IO_serial_f4()

  [DEL]     -52    PX4IO_serial_f4::_abort_dma()

  [DEL]     -52    PX4IO_serial_f4::_do_rx_dma_callback()

  [DEL]     -68    PX4IO_serial_f4::_io_buffer_storage

  [DEL]     -94    board_adc_sample

  [DEL]    -120    PX4IO_serial_f4::_do_interrupt()

  [DEL]    -154    PX4IO_serial_f4::~PX4IO_serial_f4()

  [DEL]    -168    board_adc_init

  [DEL]    -188    PX4IO_serial_f4::init()

  [DEL]    -232    PX4IO_serial_f4::ioctl()

  [DEL]    -352    PX4IO_serial_f4::_bus_exchange()

  +0.0%     +48    TOTAL

davids5

Other then noted re inclusion paths, this looks good to me

platforms/nuttx/src/px4/nxp/k66/include/px4_arch/adc.h

bkueng · 2019-08-30T05:59:38Z

Thanks for the review @davids5

bkueng force-pushed the pr_arch_specific_code_restructuring branch 3 times, most recently from bbd8610 to 1c95aaa Compare August 28, 2019 11:27

bkueng added 11 commits August 28, 2019 14:07

platform: restructure (NuttX) architecture-specific code

1c0a913

updated: tone_alarm, px4io_serial, px4_micro_hal

adc: refactor into arch-specific directories

227c65d

NuttX hrt: move into arch-specific directory

e8f6515

drv_led_pwm: move to arch-specific directory

3baa662

src/drivers/{kinetis,stm32}: move to arch-specific directories

0cd911f

ToneAlarmInterface: move to drivers/drv_tone_alarm

6d94557

It belongs there since it's not a library providing the implementation, but just declaring the interface.

src/platforms/qurt: move to src/drivers

351c8f9

platforms/posix/src: move to platforms/posix/src/px4/common

35faad7

src/platforms/posix: move to platforms/posix/src/px4/common and src/d…

eaa48a5

…rivers/driver_framework_wrapper

src/platforms/common: move to platforms/common

7ee09f6

Script to update include paths: for i in $(grep -rl 'include <px4_work_queue' src platforms); do sed -i 's/#include <px4_work_queue/#include <px4_platform_common\/px4_work_queue/' $i; done

src/platforms: move remaining source files to platforms/common

b30c1a5

bkueng force-pushed the pr_arch_specific_code_restructuring branch from 1c95aaa to 2eeba72 Compare August 28, 2019 12:07

px4_middleware: remove that header and move px4::init to px4_init.h

ae2631e

delete include: for i in $(grep -rl 'px4_middleware.h' src platforms); do sed -i '/#include <px4_middleware.h/d' $i; done

bkueng force-pushed the pr_arch_specific_code_restructuring branch from 2eeba72 to ae2631e Compare August 28, 2019 13:11

davids5 self-requested a review August 29, 2019 09:03

davids5 requested changes Aug 29, 2019

View reviewed changes

platforms/nuttx/src/px4/nxp/k66/include/px4_arch/adc.h Show resolved Hide resolved

davids5 approved these changes Aug 29, 2019

View reviewed changes

bkueng merged commit 43fdcd7 into master Aug 30, 2019

bkueng deleted the pr_arch_specific_code_restructuring branch August 30, 2019 05:59

dagar mentioned this pull request Aug 30, 2019

Increase default STACK_MAIN 1024 -> 2048 and don't set in most modules and drivers #12851

Merged

weekly-digest bot mentioned this pull request Sep 1, 2019

Weekly Digest (25 August, 2019 - 1 September, 2019) #12858

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

directory structure for architecture-specific code and headers #12821

directory structure for architecture-specific code and headers #12821

bkueng commented Aug 28, 2019

bkueng commented Aug 29, 2019

davids5 left a comment

bkueng commented Aug 30, 2019

directory structure for architecture-specific code and headers #12821

directory structure for architecture-specific code and headers #12821

Conversation

bkueng commented Aug 28, 2019

bkueng commented Aug 29, 2019

davids5 left a comment

Choose a reason for hiding this comment

bkueng commented Aug 30, 2019