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

[RFC]: board_types.txt -> consolidated board_types.h #151

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dagar
Copy link
Member

@dagar dagar commented Sep 11, 2019

I propose we merge board_types.txt into a single sorted table with all additional fields necessary to maintain compatibility between PX4, Ardupilot, and anyone else wishing to adopt this bootloader.

Note: this is a first rough pass using xmacros and only partially correct data.

Background - #138 (comment)

@dagar
Copy link
Member Author

dagar commented Sep 11, 2019

@dagar dagar force-pushed the pr-board_type_xmacro_proposal branch from 5450cca to 1af9eef Compare September 11, 2019 01:51
@tridge
Copy link
Contributor

tridge commented Sep 11, 2019

having the relevant info would be nice, but having it as pseudo-C pre-processor is pointless as it can't compile or be checked so it just makes it harder to read.
Text format is much better, and we can't do this unless the information in the file is actually accurate.
If we're going to do this then I'd also like to have it in a repo I have merge access to. I know you say now that you will actively merge my requests, but I just don't believe it as people get busy and we don't have the same priorities. We'd need a new repo, with a rep from both ardupilot and px4 having merge rights.

@dagar
Copy link
Member Author

dagar commented Sep 11, 2019

having the relevant info would be nice, but having it as pseudo-C pre-processor is pointless as it can't compile or be checked so it just makes it harder to read.
Text format is much better, and we can't do this unless the information in the file is actually accurate.

This is just something quick I hacked together to start the discussion. The idea is to have it in a form where the data can be directly used by the project (if desired). I'm fine with any tabular data if you have a suggestion.

If we're going to do this then I'd also like to have it in a repo I have merge access to. I know you say now that you will actively merge my requests, but I just don't believe it as people get busy and we don't have the same priorities. We'd need a new repo, with a rep from both ardupilot and px4 having merge rights.

As long as we're aligned on the goal of centralizing the data necessary for bootloader compatibility (not deconfliction) that's fine. I think the small amount of hassle coordinating a couple sectors here and there is worth it for the sanity of vendors and users. You can have full access here or we could start a new standalone repo that contains this alone. That might be better as each project could consume it as a submodule and avoid copying a file manually.

@pkocmoud
Copy link
Contributor

As a hardware provider it is in my best interest to preserve bootloader compatibility between both projects, I am sure that most other vendors would agree. It would be great if the build scripts could parse this file for the particular values to limit typos or errors. If it pleases everyone, possibly relocate this to an independent repo so that it can be maintained as the "authoritative open source drone IDs"?

I find the layout is easy enough to follow but I do have a couple questions.

Is there or should there be a character limit to the name?

What is Type? I see it correlates to the board ID in most instances.

Would adding a USB ID field be beneficial.

Would it be worth adding a field to indicate the applicable ports which firmware could be uploaded via, such as USB, UARTx, etc?

@dagar
Copy link
Member Author

dagar commented Sep 11, 2019

Changes (TODO)

  • remove type (can probably be dropped? Only PX4_PIO_V1 and PX4_AEROCORE_V1 differ)
  • add VENDORID
  • add PRODUCTID
  • add USBMFGSTRING? (largely still "3D Robotics")
  • add USBDEVICESTRING? (eg "PX4 BL FMU v4.x")
  • split name into fields for vendor + model

As a hardware provider it is in my best interest to preserve bootloader compatibility between both projects, I am sure that most other vendors would agree. It would be great if the build scripts could parse this file for the particular values to limit typos or errors. If it pleases everyone, possibly relocate this to an independent repo so that it can be maintained as the "authoritative open source drone IDs"?

I've created a new empty repo (https://github.com/PX4/Board_ID) we can use for this purpose. If there's consensus here then we can continue this effort in a new pull request over there.

Would it be worth adding a field to indicate the applicable ports which firmware could be uploaded via, such as USB, UARTx, etc?

This is probably beyond the minimum set of common data we need, but I could go either way.

@davids5
Copy link
Member

davids5 commented Sep 11, 2019

This is a great move in the right direction. We are going to need a bigger ID and will have to rev the bootloader to do that. Also N.B. The IO is almost out of space.

@proficnc
Copy link

We use USB VID and then PID as the identifiers.
Obviously the older black cube had the 3DR bootloader, but all new ones have our bootloader which is Chibios based. The most useful from the ground station point of view is actually the USB Device String.

In ardupilot we define all of these in the board specific hardware def files,
This makes it super easy for hardware vendors

@tridge
Copy link
Contributor

tridge commented Sep 11, 2019

This is a great move in the right direction. We are going to need a bigger ID and will have to rev the bootloader to do that.

board_type is already 32 bit. Is that the ID you're referring to?

@tridge
Copy link
Contributor

tridge commented Sep 11, 2019

I think the only string that should be in the file should be the board name, and this should not be tied to the USB strings.
The only thing we really need in common between the two bootloaders is the 32 bit board_type (called APJ_BOARD_ID in ArduPilot bootloader). This ID should map to a flash layout and that is all. The flash layout is what matters for a firmware upload tool to determine if this board is compatible with the firmware being uploaded.
The other information is out of scope for this common file I think. For ArduPilot we use a json database:
http://firmware.ardupilot.org/manifest.json
which associates board_type with all the other information, noting in particular that a single board_type can map to many different USB VID/PID,MFGSTRING etc data.
If we try to go beyond just board_type here then we open up a massive can of worms, and it really brings no benefit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants