ARMv7 build process #1024
ARMv7 build process #1024
Conversation
mikeller
left a comment
mikeller
left a comment
There was a problem hiding this comment.
Looking good so far. I hope we can solve the outstanding issues.
package.json
Outdated
| "author": "The Betaflight open source project.", | ||
| "license": "GPL-3.0", | ||
| "dependencies": { | ||
| "follow-redirects": "^1.4.1", |
There was a problem hiding this comment.
These don't seem to be used at runtime. They should be in devDependencies.
There was a problem hiding this comment.
My bad, totally forgot to move them.
There was a problem hiding this comment.
npm install --save-dev can help. 😉
gulpfile.js
Outdated
| if ((arch === 'linux32') || (arch === 'linux64') || (arch === 'armv7')) { | ||
| // Copy Ubuntu launcher scripts to destination dir | ||
| var launcherDir = path.join(folder, pkg.name, arch); | ||
| if (arch === 'armv7') { |
There was a problem hiding this comment.
I don't know too much about arm, does it need the ubuntu launcher scripts? If I'm not wrong you are generating a zip file and not an installer. Does these scripts work under arm environment?
There was a problem hiding this comment.
I've included them as a template for users to create their own desktop files to launch the Configurator, but upon second thought they should be removed - users can still create their own launchers and/or simply check the repo for a base to start with.
|
@mikeller Rebased on master. |
gulpfile.js
Outdated
| var flavorPostfix = `-${flavor}`; | ||
| var flavorDownloadPostfix = flavor !== 'normal' ? `-${flavor}` : ''; | ||
| clean_cache().then(function() { | ||
| if (!fs.existsSync('./cache')) fs.mkdirSync('./cache'); |
There was a problem hiding this comment.
Please use braces after all conditional / loop statements - this makes code a lot more maintainable.
gulpfile.js
Outdated
| targz.decompress({ | ||
| src: downloadedArchivePath, | ||
| dest: versionFolder, | ||
| }, function(err){ |
| } | ||
|
|
||
| if (platforms.indexOf('armv7') !== -1) { | ||
| releaseTasks.push(function release_armv7_zip(){ return release_zip('armv7') }); |
There was a problem hiding this comment.
If you're referring to release_armv7_zip(){, I'm consistent with the rest of the script.
There was a problem hiding this comment.
Actually you are not:
mikeller@pc:~/git/betaflight-configurator$ grep -e "function.*) {" gulpfile.js | wc -l
34
mikeller@pc:~/git/betaflight-configurator$ grep -e "function.*){" gulpfile.js | wc -l
12
And yes, the script is already inconsistent in this respect.
In general, I don't like the use of single line function definitions - they make maintenance of the code harder by increasing the probability of merge conflicts.
There was a problem hiding this comment.
In this segment of code they could as well be anonymous functions, but naming them helps a bit while debugging I guess.
I'd suggest that this block (arch-specific build functions) should be allowed to have one liners & no space after the function's declaration, in order to strive for an overall shorter line length.
If you want to keep the spaced style I'll fix the rest of the occurrences in the file, but this might somehow clutter the scope of this PR.
There was a problem hiding this comment.
We can leave them for now and clean this up in another pull request.
I do not quite get what advantage you see in 'overall shorter line length', to me the goal should be to be consistent for all function definitions.
3 participants
This PR refers to #867 and its duplicates; basing on @McGiverGim's instructions I've implemented a first draft of the ARMv7 build process.
NW.js does not ship ARM builds, but they officially endorse a specific community build. I've cloned the
linux32build process and used it to inject the ARM build of NW.js in place of thelinux32cache while packaging dependencies.This approach currently has some drawbacks:
v0.30.3, BF Configurator usesv0.28.3, and the latest ARM build isv0.27.6.armv7andlinux32targets cannot be built at the same time._ARMv7_IS_CACHEDflag file is used in the cache directory to keep track of what's in thelinux32cache).The build output is correctly recognized as
ELF 32-bit LSB pie executable ARM, EABI5 version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux-armhf.so.3, for GNU/Linux 2.6.32, stripped, and I've verified that the Configurator correctly starts up on an NTC C.H.I.P. and PocketC.H.I.P..I cannot, however, currently verify the full functionality of the Configurator because the PocketC.H.I.P.'s resolution does not allow me to actually see the whole window and/or to drag it around to reach most of the UI's buttons. As such, I would like someone's help in the following: