search

PX4 / PX4-Bootloader

PX4 Bootloader for PX4FMU, PX4IO and PX4FLOW
Other
266 stars 547 forks source link

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

Open dagar opened 5 years ago

dagar commented 5 years ago

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 - https://github.com/PX4/Bootloader/pull/138#issuecomment-530131041

dagar commented 5 years ago

@tridge @pkocmoud @davids5 @bkueng @jkflying

tridge commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Changes (TODO)

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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