Skip to content

Add custom defines support#218

Open
rob-clarke wants to merge 2 commits intoArduPilot:mainfrom
rob-clarke:add-custom-defines
Open

Add custom defines support#218
rob-clarke wants to merge 2 commits intoArduPilot:mainfrom
rob-clarke:add-custom-defines

Conversation

@rob-clarke
Copy link
Contributor

Adds a text box to the bottom of the new build page to input custom #defines. Input is in DEFINE=VALUE, DEFINE=VALUE format. These are parsed and injected into the extra_hwdef.dat file when setting up the build.

image

Any custom defines are also output in the build log:

image

Tested locally in Docker and successfully built a version with custom #defines injected. Leaving the box blank results in the current behaviour.

Addresses #217

Support adding custom defines to BuildInfo which are injected into
extra_hwdef.dat when generated.
Add text box to input custom defines to be passed to builder.
Add custom_defines to BuildRequest schema and parse custom defines.
@shiv-tyagi
Copy link
Member

Hey @rob-clarke, thanks for submitting the PR.

I honestly don't like the way the text box is placed in the form. A text area element would better suite such use case. It should also be preferably hidden.

I am also not sure if we should allow a user to add anything to the hwdef. Introducing a new feature type supporting non boolean values would make more sense. So that we present the feature name to user and user just types the value for it.

But I would like others to put their views in.

@rob-clarke
Copy link
Contributor Author

I did consider hiding the text field behind an "advanced" button or similar, probably a good idea.

The use case I have would be covered by adding an integer type, but there may be others that would benefit from a free-form input e.g. if the value should depend on another #define value (though not sure how that would work with hwdef).

I guess there could be work on adding all "configurable" #defines to build_options.py or similar to provide the metadata to enable other feature types, but that would then only be compatible with future releases.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants