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.

Sign up for GitHub

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

Jump to bottom

Added support for Unified Target configurations with manufacturer_id in the name. #1692

Merged
merged 1 commit into from Sep 28, 2019
Merged

Added support for Unified Target configurations with manufacturer_id in the name. #1692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions locales/en/messages.json
View file
Open in desktop
Expand Up @@ -2631,8 +2631,11 @@
"firmwareFlasherReleaseSummaryHead": {
"message": "Release info"
},
"firmwareFlasherReleaseName": {
"message": "Name/Version:"
"firmwareFlasherReleaseManufacturer": {
"message": "Manufacturer:"
},
"firmwareFlasherReleaseVersion": {
"message": "Version:"
},
"firmwareFlasherReleaseVersionUrl": {
"message": "Visit release page."
Expand Down
73 changes: 49 additions & 24 deletions src/js/tabs/firmware_flasher.js
View file
Open in desktop
Expand Up @@ -65,7 +65,19 @@ TABS.firmware_flasher.initialize = function (callback) {

self.enableFlashing(true);

$('div.release_info .target').text(TABS.firmware_flasher.selectedBoard);
let targetName = TABS.firmware_flasher.selectedBoard;
const TARGET_REGEXP = /^([^+-]+)(?:\+(.{1,4})|-legacy)?$/;
let targetParts = targetName.match(TARGET_REGEXP);
if (targetParts) {
targetName = targetParts[1];
if (targetParts[2]) {
$('div.release_info #manufacturerInfo').show();
$('div.release_info #manufacturer').text(targetParts[2]);
} else {
$('div.release_info #manufacturerInfo').hide();
}
}
$('div.release_info .target').text(targetName);
$('div.release_info .name').text(summary.version).prop('href', summary.releaseUrl);
$('div.release_info .date').text(summary.date);
$('div.release_info .file').text(summary.file).prop('href', summary.url);
Expand Down Expand Up @@ -264,44 +276,57 @@ TABS.firmware_flasher.initialize = function (callback) {
let releases = {};
let unifiedConfigs = {};
let items = {};
let baseTargets = {};
let unifiedTargetNames = [];
data.forEach(function(target) {
let targetName = target.name;
if (targetName.endsWith('.config')) {
targetName = targetName.slice(0,targetName.indexOf('.config'));
} else {
const TARGET_REGEXP = /^(?:([^-]{1,4})-)?(.*).config$/;
let targetParts = target.name.match(TARGET_REGEXP);
if (!targetParts) {
return;
}
unifiedConfigs[targetName]=target.download_url;
items[targetName] = "something";
let boardName = targetParts[2];
let manufacturerId = targetParts[1];
let targetName;
let displayName;
if (manufacturerId) {
targetName = `${boardName}+${manufacturerId}`;
displayName = `${boardName} (${manufacturerId})`;
} else {
targetName = boardName;
}
unifiedTargetNames.push(boardName);
unifiedConfigs[targetName] = target.download_url;
items[targetName] = { displayName: displayName };
// Chicken and egg problem: We need to know what Unified Target this configuration uses before reading the configuration.
// Solving this by assuming that all Unified Targets have the same availability for now.
const DEFAULT_UNIFIED_TARGET_NAME = "STM32F405";
releases[targetName] = builds[DEFAULT_UNIFIED_TARGET_NAME];
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that needed? for a unified target I thought it was

  1. grab the file, parse the UNIFIED_TARGET_NAME
  2. Populate versions with those releases, eg releases[STM32F411]. Otherwise the unified only targets would have shown issues.

betaflight-configurator/src/js/tabs/firmware_flasher.js

Lines 550 to 558 in 074bb46

} else {
console.log('We have the config cached for', target);
var data = storageObj.data;
bareBoard = grabBuildNameFromConfig(data);
TABS.firmware_flasher.bareBoard = bareBoard;
setUnifiedConfig(target, data, bareBoard);
populateVersions(versions_e, TABS.firmware_flasher.releases[bareBoard], target);
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Something that yields the available versions is needed.
The current way of using the non-unified .hex for this is not viable, as one goal of this whole exercise is to stop building / releasing them as soon as the Unified Target configurations are confirmed working for users, so the available versions for the non-unified targets will stop for some 4.1.x.
To get the actually used Unified Target .hex from the Unified Target config requires loading the file. If done when generating the list this will trigger GitHub's API request limiting (and be inefficient). If done when selecting a Unified Target configuration will introduce a delay and still trigger the request limit if a user is 'browsing'. So this is a pragmatic solution that will work for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Grabbing the individual config files isn't doing API calls. So I didn't worry about how many requests I do.

We're doing an API call to get the contents of configs/default (list of files) and another one for the releases. In the API call for releases, we get the last 30 releases, which is chock full of data.

If you did a release with only unified targets, users would get the proper versions, its just that the (Legacy) option wouldn't include a latest version.

Copy link
Member Author

Choose a reason for hiding this comment

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

We're doing an API call to get the contents of configs/default (list of files) and another one for the releases. In the API call for releases, we get the last 30 releases, which is chock full of data.

But we get them on a per target basis. And if we use a target that has been replaced by a Unified Target configuration and isn't released any more to populate the Unified Target configuration's releases, there won't be any releases showing for after we've pulled the non-unified target from release.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you download the releases, and remove the 4.1.0 RC builds for one target it looks like this:
image
This PR
image
Master
image

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, that is what I am going for - the aim is to phase out the non-unified targets in releases going forward (i.e. only Unified Targets and 'exotic' non-unified targets should be included in later 4.1.x releases).
For now, having access to the legacy targets provides a fallback for users if there are problems with Unified Targets, but in a future release we'll probably want to merge them into the 'normal' target selection, and give users non-unified targets for releases before 4.1.0-RC1, and Unified Targets after.
At this point we should also look into switching to using releases for the actual Unified Target .hex for every target - right now I am just not sure how best to do this. Speculatively loading all 140 (for now) Unified Target configurations seems to be a non-ideal way. Maybe we can generate a file containing a list of all Unified Target configurations with their required Unified Target .hex as part of the CI process in the unified-targets repository?

Copy link
Contributor

Choose a reason for hiding this comment

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

Generating a file sounds good, if we do it as part of the CI process, where does it go? Oh we have options for that. My vote is github pages https://docs.travis-ci.com/user/deployment/pages/

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking of a future CI process that would push any changes to master to something that is more suited as a content delivery network as well.
But even without this, or having CI modify the repository in any form, adding a script that re-generates the index file, requiring target maintainers to update the index file (by script or manually) as part of every pull request, and using CI to fail pull requests that fail to do so should work just fine as a first incarnation.

});
Object.keys(builds).forEach(function (key) {
// releases is under the hood, so we can have duplicate entries
var legacyKey = key + " (Legacy)";
if (unifiedConfigs[key] === undefined) {
items[key] = "something";
releases[key] = builds[key];
let targetName;
let displayName;
if (unifiedTargetNames.includes(key)) {
targetName = `${key}-legacy`;
displayName = i18n.getMessage("firmwareFlasherLegacyLabel", { target: key });
} else {
items[legacyKey] = "i18nplz";
baseTargets[legacyKey] = key;
releases[legacyKey] = builds[key];
releases[key] = builds[key];
targetName = key;
}
items[targetName] = { displayName: displayName };
releases[targetName] = builds[key];
});
$('select[name="board"]').empty()
var boards_e = $('select[name="board"]');
var versions_e = $('select[name="firmware_version"]');
boards_e.empty()
.append($("<option value='0' i18n='firmwareFlasherOptionLabelSelectBoard'></option>"));

$('select[name="firmware_version"]').empty()
versions_e.empty()
.append($("<option value='0' i18n='firmwareFlasherOptionLabelSelectFirmwareVersion'></option>"));
var boards_e = $('select[name="board"]');
var versions_e = $('select[name="firmware_version"]');
var selectTargets = [];
Object.keys(items)
.sort()
.forEach(function(target, i) {
var select_e = $("<option value='{0}'>{1}</option>".format(target,
items[target] === "i18nplz" ? i18n.getMessage("firmwareFlasherLegacyLabel",
{target: baseTargets[target]}) : target));
let item = items[target];

var select_e = $("<option value='{0}'>{1}</option>".format(target, items[target].displayName || target));
boards_e.append(select_e);
});
TABS.firmware_flasher.releases = releases;
Expand Down
24 changes: 18 additions & 6 deletions src/tabs/firmware_flasher.html
View file
Open in desktop
Expand Up @@ -88,12 +88,24 @@
i18n="firmwareFlasherReleaseSummaryHead"></div>
</div>
<div class="spacer" style="margin-bottom: 10px;">
<strong i18n="firmwareFlasherReleaseTarget"></strong> <span class="target"></span><br /> <strong
i18n="firmwareFlasherReleaseName"></strong> <a i18n_title="firmwareFlasherReleaseVersionUrl" class="name"
href="#" target="_blank"></a><br /> <strong i18n="firmwareFlasherReleaseFile"></strong> <a
i18n_title="firmwareFlasherReleaseFileUrl" class="file" href="#" target="_blank"></a><br /> <strong
i18n="firmwareFlasherReleaseDate"></strong> <span class="date"></span><br /> <strong
i18n="firmwareFlasherReleaseNotes"></strong>
<strong i18n="firmwareFlasherReleaseTarget"></strong>
<span class="target"></span>
<br />
<div id="manufacturerInfo">
<strong i18n="firmwareFlasherReleaseManufacturer"></strong>
<span id="manufacturer"></span>
<br />
</div>
<strong i18n="firmwareFlasherReleaseVersion"></strong>
<a i18n_title="firmwareFlasherReleaseVersionUrl" class="name" href="#" target="_blank"></a>
<br />
<strong i18n="firmwareFlasherReleaseFile"></strong>
<a i18n_title="firmwareFlasherReleaseFileUrl" class="file" href="#" target="_blank"></a>
<br />
<strong i18n="firmwareFlasherReleaseDate"></strong>
<span class="date"></span>
<br />
<strong i18n="firmwareFlasherReleaseNotes"></strong>
<div class=notes></div>
</div>
</div>
Expand Down