Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

boards organization #10875

Merged
merged 17 commits into from
Nov 26, 2018
Merged

boards organization #10875

merged 17 commits into from
Nov 26, 2018

Conversation

dagar
Copy link
Member

@dagar dagar commented Nov 20, 2018

As previously discussed in conjunction with #7705, #8777, #7806, #8127, #7318, #8923, I'd like to consolidate all components of a board into a single location.

Why?

  • effectively creates a board "API" showing all pieces that need to be implemented for a new board
  • easy board addition
  • easy board removal
  • third party boards can live out of tree
  • direct comparison between boards (ie diff fmu-v2 and fmu-v4)
  • trivial to create a custom minor board variant without hacking the existing board in a private fork (something I've seen many times).
  • work towards minimizing what's actually required in a board support package

Example

  • fmu-v2
    • default.cmake
    • nuttx-config
    • include
      • board_config.h
    • src
      • init.c
      • etc

In this current PR I've sorted the boards by VENDOR (eg PX4) and then by MODEL (eg FMU-v2). The downside is it changes the common naming from px4fmu-v2 to px4_fmu-v2. This is somewhat flexible if people feel strongly about it.

Thoughts/comments/concerns/complaints?

% tree -L 3 --dirsfirst boards

boards
├── aerotenna
│   └── ocpoc
│       ├── src
│       └── ubuntu.cmake
├── airmind
│   └── mindpx-v2
│       ├── nuttx-config
│       ├── src
│       ├── default.cmake
│       └── firmware.prototype
├── atlflight
│   ├── cmake_hexagon
│   ├── eagle
│   │   ├── src
│   │   ├── default.cmake
│   │   └── qurt-default.cmake
│   └── excelsior
│       ├── src
│       ├── default.cmake
│       └── qurt-default.cmake
├── atmel
│   └── same70xplained
│       ├── nuttx-config
│       ├── src
│       ├── default.cmake
│       └── firmware.prototype
├── auav
│   ├── esc35-v1
│   │   ├── nuttx-config
│   │   ├── src
│   │   ├── default.cmake
│   │   └── firmware.prototype
│   └── x21
│       ├── nuttx-config
│       ├── src
│       ├── default.cmake
│       └── firmware.prototype
├── av
│   └── x-v1
│       ├── nuttx-config
│       ├── src
│       ├── default.cmake
│       └── firmware.prototype
├── beaglebone
│   └── blue
│       ├── src
│       ├── cross.cmake
│       └── native.cmake
├── bitcraze
│   └── crazyflie
│       ├── nuttx-config
│       ├── src
│       ├── default.cmake
│       └── firmware.prototype
├── emlid
│   └── navio2
│       ├── src
│       ├── cross.cmake
│       └── native.cmake
├── gumstix
│   └── aerocore2
│       ├── nuttx-config
│       ├── src
│       ├── default.cmake
│       └── firmware.prototype
├── intel
│   └── aerofc-v1
│       ├── nuttx-config
│       ├── src
│       ├── default.cmake
│       ├── firmware.prototype
│       └── rtps.cmake
├── nxp
│   └── hlite-v3
│       ├── nuttx-config
│       ├── src
│       ├── default.cmake
│       └── firmware.prototype
├── omnibus
│   └── f4sd
│       ├── nuttx-config
│       ├── src
│       ├── default.cmake
│       └── firmware.prototype
├── parrot
│   └── bebop
│       ├── src
│       └── default.cmake
├── px4
│   ├── cannode-v1
│   │   ├── nuttx-config
│   │   ├── src
│   │   ├── default.cmake
│   │   └── firmware.prototype
│   ├── esc-v1
│   │   ├── nuttx-config
│   │   ├── src
│   │   ├── default.cmake
│   │   └── firmware.prototype
│   ├── fmu-v2
│   │   ├── nuttx-config
│   │   ├── src
│   │   ├── default.cmake
│   │   ├── firmware.prototype
│   │   ├── lpe.cmake
│   │   └── test.cmake
│   ├── fmu-v3
│   │   ├── nuttx-config
│   │   ├── src
│   │   ├── default.cmake
│   │   ├── firmware.prototype
│   │   ├── rtps.cmake
│   │   └── stackcheck.cmake
│   ├── fmu-v4
│   │   ├── nuttx-config
│   │   ├── src
│   │   ├── default.cmake
│   │   ├── firmware.prototype
│   │   ├── rtps.cmake
│   │   └── stackcheck.cmake
│   ├── fmu-v4pro
│   │   ├── nuttx-config
│   │   ├── src
│   │   ├── default.cmake
│   │   ├── firmware.prototype
│   │   └── rtps.cmake
│   ├── fmu-v5
│   │   ├── nuttx-config
│   │   ├── src
│   │   ├── default.cmake
│   │   ├── firmware.prototype
│   │   ├── rtps.cmake
│   │   └── stackcheck.cmake
│   ├── io-v2
│   │   ├── nuttx-config
│   │   ├── src
│   │   ├── default.cmake
│   │   └── firmware.prototype
│   ├── raspberrypi
│   │   ├── src
│   │   ├── cross.cmake
│   │   └── native.cmake
│   └── sitl
│       ├── src
│       ├── default.cmake
│       ├── rtps.cmake
│       └── test.cmake
├── stm
│   ├── 32f4discovery
│   │   ├── nuttx-config
│   │   ├── src
│   │   ├── default.cmake
│   │   └── firmware.prototype
│   └── nucleo-F767ZI
│       ├── nuttx-config
│       ├── src
│       ├── default.cmake
│       └── firmware.prototype
└── thiemar
    └── s2740vc-v1
        ├── nuttx-config
        ├── src
        ├── default.cmake
        └── firmware.prototype

97 directories, 67 files

@LorenzMeier
Copy link
Member

Very cool?

Copy link
Member

@davids5 davids5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dagar I have no issue with the command name changes. I do question the CONFIG_BOARD_NAME changes. What it that motivation? if it has to be can it have a "_" to seperate the vendor and board in it?

@dagar
Copy link
Member Author

dagar commented Nov 20, 2018

At the moment CONFIG_BOARD_NAME has to be consistent with BOARD at the PX4 level. I don't really care for it either, but once we transition to nuttx custom boards (instead of tricking it by copying into the tree) it will go away.

@davids5
Copy link
Member

davids5 commented Nov 20, 2018

At the moment CONFIG_BOARD_NAME has to be consistent with BOARD at the PX4 level. I don't really care for it either, but once we transition to nuttx custom boards (instead of tricking it by copying into the tree) it will go away.

fair enough. What do we need to do to get this in?

@mrpollo
Copy link
Contributor

mrpollo commented Nov 20, 2018

Can we define the testing that needs to happen before this gets merged?

@dagar dagar force-pushed the pr-boards branch 2 times, most recently from e946813 to cc4f554 Compare November 20, 2018 19:30
@davids5
Copy link
Member

davids5 commented Nov 20, 2018

I have tested on v2 HW with uavcan downloading px4cannode. That is all working.

Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, looks good.

image: docker_images.nuttx,
archive: false
]

def rpi_builds = [
target: ["posix_rpi_cross", "posix_bebop_default"],
target: ["emlidnavio2_cross", "parrotbebop_default"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit strange that the company and board are not separated. Is there a reason? I.e. emlid-navio2_cross would already help.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main reason it's been kept like this so far is to minimize migration pain.

 px4fmu-v5_default -> px4_fmu-v5_default

I'd still like to split, but we'll need everyone onboard so that we can do a coordinated documentation update.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My lets fix this once and for all. Let's move forward with the name change. Provide an "is" "was" list in the commit and we can all adjust or fix what it breaks.

.travis.yml Outdated
@@ -28,5 +28,5 @@ addons:
description: "Build submitted via Travis CI"
notification_email: ci@px4.io
build_command_prepend: "make distclean"
build_command: "make posix_sitl_default"
build_command: "make px4sitl_default"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need docs update (@hamishwillee fyi)

@hamishwillee hamishwillee added the Documentation 📑 Anything improving the documentation of the code / ecosystem label Nov 22, 2018
@dagar
Copy link
Member Author

dagar commented Nov 22, 2018

I've pushed a commit to change the naming convention to include an underscore between the vendor and model. The platform prefix is also no longer necessary.

posix_sitl_default -> px4_sitl_default
px4fmu-v2_default -> px4_fmu-v2_default

@dagar dagar force-pushed the pr-boards branch 2 times, most recently from 7620ad0 to da590a3 Compare November 22, 2018 04:58
@Avysuarez
Copy link

Tested on pixhawk 4 and pixhawk 4 mini. No issues, Good flights.
Pixhawk 4.
https://review.px4.io/plot_app?log=a01ee82e-fe67-4755-98d8-649cc51ecd02
Pixhawk 4 mini.
https://review.px4.io/plot_app?log=8b8adf6b-0bcb-49c0-a684-0f23685c4e71

@dannyfpv
Copy link

Copy link
Member

@davids5 davids5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dagar Nice cleanup.

@mrpollo
Copy link
Contributor

mrpollo commented Nov 27, 2018

👏 👏 👏 👏 👏 👏 👏 👏 👏
This community is awesome, thanks for the cleanup guys.

@dagar 13k deletions ❤️

👏 👏 👏 👏 👏 👏 👏 👏 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Admin: Enhancement (improvement) 💡 Documentation 📑 Anything improving the documentation of the code / ecosystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants