Cordova framework to build Android app #1946
Conversation
WalcoFPV
changed the title
|
Hi! Great work. I don't know too much about Cordova, so I can't make a good review to this PR, but let's see first what is the general consensus about adding this and then we can start with the review. Only one thing: I've seen you have uploaded the |
There was a problem hiding this comment.
This looks very interesting, thank you for your contribution. We are now in the release candidate phase for a firmware release, so this will have to wait until Betaflight 4.2.0 has been released, and I will do a more thorough review then.
In the meantime I have pointed out some big picture items that will have to be done to make this maintainable, and there are also the issues reported by Sonar that you can look at fixing.
|
Happy to see that my work interests you. I will try to make all the configurator functions work on Android for the next Betaflight release. In my last commit, I created a new file |
|
Awesome, thanks for this. Finally all features are working like Firmware flashing?? Can you share a test.apk please?? |
|
https://drive.google.com/open?id=1tPS85Z5u8xCFsuekUBll-htYRZt-AJw0 |
|
Nice thanks, firmware flasher is very interesting, actually we only have one apk for it "Drone firmware flasher". |
|
I see you are fixing Sonar issues... only one thing: the objective is to have 0 issues, but sometimes, some issues can be left if fixing them is complicated or the resulting code is worst than original (some complexity factor reduction, or some change strings by consts...). |
|
I began to work on the ui for phones. I adapted the header and the tab list. The content of tabs is not adapted yet. |
|
Here we are. Almost all the configurator works on and is adapted to phones. Currently, only the firmware flasher doesn't work. It requires the creation of a Cordova plugin coded with Java and I don't have the required skills. I provide you more screenshots and an apk file if you want to try the app. https://drive.google.com/drive/folders/1tPS85Z5u8xCFsuekUBll-htYRZt-AJw0?usp=sharing |
|
As you well know @WalcoFPV it seems an incredible contribution. The phone adaptation looks great and I'm glad we can soon enjoy this official Betaflight apk. Again thank you so much for making it happens. |
|
Great work @WalcoFPV - having Betaflight configurator available on phones to allow in-field tuning will be a great improvement! I don't think that not supporting firmware flashing will be a major issue for this use case. |
|
This should work with iOS via Bluetooth right? |
|
@joevo2 No this should actually work only on Android via OTG. It could be relatively easy to add bluetooth support but I don't have any bluetooth module or fc with integrated bluetooth. But IOS will be more difficult to support due to restrictions of ios |
|
Amazing~! |
|
Rebased onto 10.7.0 RC2 |
|
@mikeller @McGiverGim is there a way to clean up the cache of Travis CI ? I tried with cache disabled and the check was passed but with cache enabled, it failed |
There was a problem hiding this comment.
Only some comments or questions.
Is a great, great job. But is a very big, big PR. You have fixed inside a lot of little issues that are outside of this PR too, but I think that now we can't take them out with a lot of effort, so for me is ok to merge the complete PR.
I will do some tests more, to try to find if we break something that now works, but again, great, great job. A lot of users will be happy with this.
|
Strange that now Sonar detects the code smells. When you have time please take a look to them, specially the one about declaring functions inside a loop. The preview has been downloaded more than 2000 times... not bad being a preview only :) more than the Mac OS version or some of the Linux versions: |
|
@McGiverGim Seems that Sonar doesn't analyse any css or js code inside an html file. Moreover, the app seems to be pretty stable for a preview :) The issue found recently seems to be caused by a cordova plugin and not by the content of this PR |
|
@McGiverGim I found an issue with the keystore so I generated a new one. And I just changed the cordova plugins url and squashed commits |
|
Rebased and fixed this issue #2061 |
| * **release** zips up the apps into individual archives in the `./release` folder [1]. | ||
|
|
||
| [1] Running this task on macOS or Linux requires Wine, since it's needed to set the icon for the Windows app (build for specific platform to avoid errors). | ||
| [2] For Android platform, **dist** task will generate the `./cordova` folder |
There was a problem hiding this comment.
Nitpick - this does not seem to be true any more - probably rather 'or Android platform, dist task will generate folders and files in the ./cordova folder'.
A cleaner way to do this would be to have these files generated under /dist or /cordova-dist, but not sure if this is possible in an easy way with Cordova.
There was a problem hiding this comment.
I can rename the cordova folder to cordova-dist. But I think, there is a risk of confusion, the equivalent of the dist folder in the cordova project is more cordova/src. Moreover, the cordova folder can not be deleted like the dist folder
There was a problem hiding this comment.
Part of the problem is that the statement 'For Android platform, dist task will generate the ./cordova folder' will make people think that this directory can be deleted when cleaning up after a build. Another problem is that yarn gulp clean currently fails to remove build artefacts in /cordova - the easiest solution for this will be to separate the files that currently end up under /cordova into two different directories: One for the permanent files, another one (which can be a sub-directory) for the build artefacts. This will also make fixing yarn gulp clean a lot easier.
But this can all be done in a follow-up pull request.
There was a problem hiding this comment.
@WalcoFPV: I think this needs to be fixed with some urgency, and it is a little bit more complex than previously thought:
Since a build of the Android app modifies some files that are under source control (cordova/config.xml, cordova/config.xml) these will appear as changed and sneak into subsequent commits for unrelated changes:
This is a hassle and will require a lot of manual cleanup in the future, so it should be fixed - probably best rename the files to ..._template or suchlike, and create a copy of them in the directory that is to contain the build artefacts.
|
Should I fix the code smells ? These are not code smells due to my changes |
|
No, if they are not from your changes you don't need to fix them. Some of them usually will be fixed when you rebase your code. |
Cordova integration and android platform : - Added cordova directory with required config - Added cordova applications generation in gulpfile - Added cordova development instructions - Used cordova plugins to simulate missing chrome api plugins (chrome.serial and chrome.fileSystem) - Added cordova clipboard support - Added android operating system and Cordova gui mode - Fixed some css and js files to make them working on Android as well as on computers - Added --skipdep argument to accelerate cordova build (gulp task) - Added a webview helper to help people to update the webview app of their device New options tab : - Added options tab replacing the options dropdown - Added option to switch between phones UI and computers UI Mobile interface and global interface improvements : - Simplified the structure of the header with flex css - Made headerbar and tab container responsive (compact headerbar and side menu) - All tabs are adapted to mobile interface (except firmware flasher) - The servos and adjustments tabs are not fully adapted but are "usable" - Improved header bar animation - Improved log expandation animation - Added swipe gesture to toggle side menu Fixes during the development : - Logo position - Dark mode - Auto connection - Error messages (cordova_chromeapi.js) - Responsive grid - Testing - Disconnection - Width of boxes inside the OSD tab - Fixed cli tab - OSD tab - Motor stop switch - White spaces in boxes - Dialogs size - Connect button state - Prevent tablet with a height larger than 575px to switch to computers ui - Fixed logging tab - Fixed code smell - Fixed yarn cordova plugin install issue - Fixed content_wrapper - Fixed vibrations when scrolling - Fixed scrolling bar alignment - Fixed dialogReportProblem height - Fixed rates logo - Fixed auto connection default value (true) - Fixed D to D max - Fixed dialogs Added required messages in locales/en/messages.json file Requested changes
|
Kudos, SonarCloud Quality Gate passed!
|
|
@WalcoFPV: One more thing I noticed when testing with this is that currently, the Android version gets the version information from two different sources:
|
This will allow building an Android app.
The folder
./cordovacontains all Cordova related files. There are some minor changes on the Configurator to fix bug.Which are links between the standard version and the Cordova version of the configurator ?
There are only 2 files different depending on the standard version and the cordova version:
./src/js/startup_cordova.js|./src/js/startup.js./src/js/serial_cordova.js|./src/js/serial.jsAll other files are common to both versions.
By default, the changes do not affect the standard version of the configurator. It is during the generation with gulp that changes will take effect. The
./cordova/wwwfolder generation, building, debuging and releasing are fully integrated in the gulpfile. Cordova building is not integrated in Travis CI yet.On the Android app, the firmware flasher is disabled because the Chrome API usb doesn't work properly. There are also issues related to functions which required to read or to write a file. For example, "Save to file" button in CLI. I did not find other issues at this moment.
This build is not fully stable yet. But I decided to open this pull request to discuss, mainly about the global integration of Cordova.