betaflight
betaflight copied to clipboard
Extend build info with defined flags
This PR extends the MSP_BUILD_INFO
command response with a few bytes as a way to make the firmware able to communicate the build options it was built with to clients, thus eliminating the need to have a working internet connection to retrieve this info. One of the main motivations for this is to make the Configurator work better when offline and make the TX lua scripts more efficient by knowing what information it can retrieve and display. Hopefully this will make the TX setup tool work much faster. The current list of options was retrieved from this endpoint.
Do you want to test this code? You can flash it directly from Betaflight Configurator:
- Simply put
#13333
(this pull request number) in theSelect commit
field of the Configurator firmware flasher tab (you need toEnable expert mode
,Show release candidates
andDevelopment
).
WARNING: It may be unstable. Use only for testing!
Another possible implementation:
- augment https://build.betaflight.com/api/options/4.5.0-zulu with group and option id's (small integer that is not reused on protocol changes)
- add python (or perl) code generator to BF, together with generated files (and preferable add makefile rules)
- use slightly more general structure for encoding, for example (
<group-id:5bits; group-lenght-bytes:3bits><group bitdata><next-group-id+length ... ><0x00>
) - state that
<group bitdata>
may be defined specially for given group-id. Configurator will skip unknown group-ids / more efficient encoding for group-id may be defined in future
Sorry guys this has to be a 4.6 feature I think.
Another possible implementation:
- augment https://build.betaflight.com/api/options/4.5.0-zulu with group and option id's (small integer that is not reused on protocol changes)
- add python (or perl) code generator to BF, together with generated files (and preferable add makefile rules)
- use slightly more general structure for encoding, for example (
<group-id:5bits; group-lenght-bytes:3bits><group bitdata><next-group-id+length ... ><0x00>
)- state that
<group bitdata>
may be defined specially for given group-id. Configurator will skip unknown group-ids / more efficient encoding for group-id may be defined in future
I wonder if - given the sprawl of options will likely far exceed the options actually selected by the user - we simply assign a 2 byte number to each option (could be more efficient, but 1 byte is insufficient) and then simply stream these in the MSP response. We could as you have suggested assign a grouping for the first few bits (RX, GPS, MOTOR etc).
Only those selected will end up compiled, streamed and taking up flash / MSP message space.
Would also make the CLI output of the same human readable (at least numerically).
Thank you for your valuable input and inspiring ideas! I know currently this is a very simplistic approach, but my most important concern was the amount of bytes transferred with over-the-air usage in mind to make configuration on the field more responsive by reducing the time to initialize and reduce confusion by hiding unavailable options. Currently we can only do this by firing an MSP request per feature and wait for an error or success response and it is far from efficient so retrieving the list of possible options in one go will help a lot.
@ledvinap You are right, making it possible to generate code for this purpose was part of the reasons why I moved this into its separate file, so it is naturally doable. However it will introduce its own problems in the build tooling of course.
I really like the idea of skippable groups (e.g. it's likely that "telemetry" options will be often skipped due to the popularity of CRSF), spending a header byte to indicate included groups seem worthy. Currently we have 4 groups, is it likely we'll ever extend beyond 8 groups? Also spending 2 bytes per gate feels like an overkill. Say the user will likely select at least 5-6 gates (currently there are 9 gates set as default if I'm not mistaken) which is already 10-18 bytes while the code in this PR would just send a fixed 6 bytes for all 43 gates (7, 8 .. bytes once we extend beyond 48, 56 .. gates).
Although it is also true that @blckmn 's idea would make the protocol really simple in contrast to the more general group structure proposal. At the moment I'm failing to see a good use case for letting the internal group data a custom structure depending on the group, considering that gates are really just true/false values. To use the response implemented in this PR could be processed on the lua side as simply as the following:
// test availability of OSD SD
local dateLength = 11
local timeLength = 8
local revisionLength = 7
// payload is the MSP response
local optionByte4 = payload[dateLength + timeLength + revisionLength + 4]
local hasOSDSD = bit32.band(optionByte4, 0x07) ~= 0
But of course the drawback of using arbitrarily ordered values remains. I'm totally open to improve on this simplistic approach, but somewhat confused about how to balance between a more sane structure and still keeping a minimal payload size, and perhaps not complicating encoding/decoding too much either...
I would not worry too much about payload size. It's a once per connection request response.
I'm more concerned with flash space and maintainability.
I would not worry too much about payload size. It's a once per connection request response.
I'm more concerned with flash space and maintainability.
Fine, that sounds convincing enough :) In this case I'm planning to go with the 2 byte indices approach, if that's okay
@atomgomba:
Something line this may work:
static const uint16_t build_options[] = {
#ifdef USE_SERIALRX_FPORT
BUILD_OPTION_SERIALRX_FPORT,
#endif
#ifdef ...
};
And then use ARRAYLEN() to send build_options
...
@atomgomba:
Something line this may work:
static const uint16_t build_options[] = { #ifdef USE_SERIALRX_FPORT BUILD_OPTION_SERIALRX_FPORT, #endif #ifdef ... };
And then use ARRAYLEN() to send
build_options
...
Exactly this. The header file setting the BUILD_OPTION_SERIALRX_FPORT value can be generated (at least initially), and I will update the API to also return the "values" when https://build.betaflight.com/api/options/4.5.0 or similar is obtained.
Yes, I think we're on the same page. Currently I have this sketch (please see last commit), not tested yet. I used this script for generating the code. I realized if I used defines instead of an enum then unused values won't be included in the compiled binary, if I'm correct. EDIT: Or would the compiler optimise for size and it won't matter anyway?
Defines are fine. Please note I have added the keys (which includes the grouping - using 4 high order bits, allowing for up to 15 groups, and 4095 total options) into the API, and so you can simply use those values.
see here: https://build.betaflight.com/api/options/4.5.0
// Radio Protocols
#DEFINE BUILD_OPTION_SERIALRX_CRSF 4097
#DEFINE BUILD_OPTION_SERIALRX_FPORT 4098
#DEFINE BUILD_OPTION_SERIALRX_GHST 4099
#DEFINE BUILD_OPTION_SERIALRX_IBUS 4100
#DEFINE BUILD_OPTION_SERIALRX_JETIEXBUS 4101
#DEFINE BUILD_OPTION_RX_PPM 4102
#DEFINE BUILD_OPTION_SERIALRX_SBUS 4103
#DEFINE BUILD_OPTION_SERIALRX_SPEKTRUM 4104
#DEFINE BUILD_OPTION_SERIALRX_SRXL2 4105
#DEFINE BUILD_OPTION_SERIALRX_SUMD 4106
#DEFINE BUILD_OPTION_SERIALRX_SUMH 4107
#DEFINE BUILD_OPTION_SERIALRX_XBUS 4108
// Telemetry Protocols
#DEFINE BUILD_OPTION_TELEMETRY_FRSKY_HUB 12301
#DEFINE BUILD_OPTION_TELEMETRY_HOTT 12302
#DEFINE BUILD_OPTION_TELEMETRY_IBUS_EXTENDED 12303
#DEFINE BUILD_OPTION_TELEMETRY_LTM 12304
#DEFINE BUILD_OPTION_TELEMETRY_MAVLINK 12305
#DEFINE BUILD_OPTION_TELEMETRY_SMARTPORT 12306
#DEFINE BUILD_OPTION_TELEMETRY_SRXL 12307
// General Options
#DEFINE BUILD_OPTION_ACRO_TRAINER 16404
#DEFINE BUILD_OPTION_AKK_SMARTAUDIO 16405
#DEFINE BUILD_OPTION_BATTERY_CONTINUE 16406
#DEFINE BUILD_OPTION_CAMERA_CONTROL 16407
#DEFINE BUILD_OPTION_DASHBOARD 16408
#DEFINE BUILD_OPTION_EMFAT_TOOLS 16409
#DEFINE BUILD_OPTION_ESCSERIAL_SIMONK 16410
#DEFINE BUILD_OPTION_FRSKYOSD 16411
#DEFINE BUILD_OPTION_GPS 16412
#DEFINE BUILD_OPTION_LED_STRIP 16413
#DEFINE BUILD_OPTION_LED_STRIP_64 16414
#DEFINE BUILD_OPTION_MAG 16415
#DEFINE BUILD_OPTION_OSD_SD 16416
#DEFINE BUILD_OPTION_OSD_HD 16417
#DEFINE BUILD_OPTION_PINIO 16418
#DEFINE BUILD_OPTION_RACE_PRO 16419
#DEFINE BUILD_OPTION_SERVOS 16420
#DEFINE BUILD_OPTION_VTX 16421
// Motor Protocols
#DEFINE BUILD_OPTION_BRUSHED 8230
#DEFINE BUILD_OPTION_DSHOT 8231
#DEFINE BUILD_OPTION_MULTISHOT 8232
#DEFINE BUILD_OPTION_ONESHOT 8233
#DEFINE BUILD_OPTION_PROSHOT 8234
#DEFINE BUILD_OPTION_PWM_OUTPUT 8235
What do you think about the latest changes? I would like to make the changes in the lua scripts to take advantage of this, even if this won't be merged to master right away, I just would like to avoid back-and-forth modifications.
While testing some fields are missing (for current logic):
Can we add:
USE_TELEMETRY_CFRF
USE_TELEMETRY_GHST
USE_TELEMETRY_FPORT
USE_TRANSPONDER
Maybe we also add
USE_RX_MSP
USE_RX_PARALLEL_PWM
USE_SPI_RX
USE_RX_SERIAL
Configurator depends on these to be available in features.js:
const features = [
{bit: 0, group: 'rxMode', mode: 'select', name: 'RX_PPM'},
{bit: 2, group: 'other', name: 'INFLIGHT_ACC_CAL'},
{bit: 3, group: 'rxMode', mode: 'select', name: 'RX_SERIAL'},
{bit: 4, group: 'escMotorStop', name: 'MOTOR_STOP'},
{bit: 5, group: 'other', name: 'SERVO_TILT', haveTip: true, dependsOn: 'SERVOS'},
{bit: 6, group: 'other', name: 'SOFTSERIAL', haveTip: true},
{bit: 7, group: 'other', name: 'GPS', haveTip: true, dependsOn: 'GPS'},
{bit: 9, group: 'other', name: 'SONAR', haveTip: true, dependsOn: 'RANGEFINDER'},
{bit: 10, group: 'telemetry', name: 'TELEMETRY', haveTip: true, dependsOn: 'TELEMETRY'},
{bit: 12, group: '3D', name: '3D', haveTip: true},
{bit: 13, group: 'rxMode', mode: 'select', name: 'RX_PARALLEL_PWM'},
{bit: 14, group: 'rxMode', mode: 'select', name: 'RX_MSP'},
{bit: 15, group: 'rssi', name: 'RSSI_ADC'},
{bit: 16, group: 'other', name: 'LED_STRIP', haveTip: true, dependsOn: 'LED_STRIP'},
{bit: 17, group: 'other', name: 'DISPLAY', haveTip: true, dependsOn: 'DASHBOARD'},
{bit: 18, group: 'other', name: 'OSD', haveTip: true, dependsOn: 'OSD'},
{bit: 20, group: 'other', name: 'CHANNEL_FORWARDING', dependsOn: 'SERVOS'},
{bit: 21, group: 'other', name: 'TRANSPONDER', haveTip: true, dependsOn: 'TRANSPONDER'},
{bit: 22, group: 'other', name: 'AIRMODE'},
{bit: 25, group: 'rxMode', mode: 'select', name: 'RX_SPI'},
{bit: 27, group: 'escSensor', name: 'ESC_SENSOR'},
{bit: 28, group: 'antiGravity', name: 'ANTI_GRAVITY', haveTip: true, hideName: true},
];
I guess these are the ones that aren't part of auto-complete as they have their own UI widget for selecting them. Once they become included in the API, I can regenerate the list of defines :+1:
@atomgomba : Can you please create PR for generator scripts?
Master:
This PR and configurator PR:
@atomgomba : Can you please create PR for generator scripts?
Sure thing, but it will need some more work to be truly useful. At the moment it just writes enumerations to files in the same directory and I copy-pasted the contents into the C source files. I'm guessing the best would be for it to reside under src/utils
and directly replace msp_build_info.*
files upon execution.
Can we have support for
https://github.com/betaflight/betaflight-configurator/pull/3583, meaning we need some groups as @haslinghuis comment, to support higerlevel groups the in
ie.
// Radio Protocols
#DEFINE BUILD_GROUP_RADIO 4000
// Telemetry Protocols
#DEFINE BUILD_GROUP_TELEMETRY 12000
// General Options
#DEFINE BUILD_GROUP_GENERAL 16000
// Motor Protocols
#DEFINE BUILD_GROUP_MOTOR 8200
@HThuren Could you please elaborate? I don't understand why including the options that serve as input parameters to make
isn't enough
The group value is already contained in the value:
@atomgomba, right, @haslinghuis show me groups, I can see they already are there. My point of view are to support when you setup in configurator, and here to help that user can't features not supported in build.
I will hope to have more groups, ie General splitted more, to cover, "Your solution" / "Suggestion" :
@HThuren Please note that Configurator-over-the-wire isn't the only potential consumer of this MSP reply. The response is already too verbose to my linking (originally I imagined transferring only a handful of bytes containing bit flags), I don't think we should send anything that can be included/calculated on the client side. OTA data transfer can be very slow already on some protocols and the whole point is to avoid sending one MSP command per feature to test their availability. If a single response becomes nearly as long as multiple other ones combined then it will defeat the whole purpose.
@atomgomba maybe for LUA [we] need to introduce another MSP request for retrieving a specific build option instead of retrieving the whole list.
@atomgomba maybe for LUA [we] need to introduce another MSP request for retrieving a specific build option instead of retrieving the whole list.
I'm sold. This is the way!
This is the way!
This last pushed script accepts an option for the URL to the endpoint (or defaults to the current one when omitted), and generates identical source files to the ones in this PR in place.
One more thing: Generated files should contain info about data source. Using only endpoint may be enough, but better info will be better.
@ledvinap Thank you for the review and valuable comments!
I replaced the timestamp with a stable hash of the input data