Add NXT support for GNU/Linux #1696
Conversation
zhitm
force-pushed
the
master
branch
4 times, most recently
from
c4b5f98
to
f828e2b
Compare
| trikV62QtsGenerator \ | ||
| trikPythonGeneratorLibrary \ | ||
| trikV62PythonGenerator \ | ||
| trikGeneratorBase \ |
| case QOperatingSystemVersion::OSType::Windows: { | ||
| QFile compile1(dir.absolutePath() + "/compile.bat"); | ||
| QFile compile2(dir.absolutePath() + "/compile.sh"); | ||
| mNxtToolsPresent = gnuarm.exists() && nexttool.exists() && nxtOSEK.exists() |
There was a problem hiding this comment.
The preferable way to check for existence of the FS object is QFileInfo::exists(path). Also, in case you need a readable regular file, the exists is not enough, because it returns true for directories too.
| setColorOnGeneratorLabel(QColor("red")); | ||
| } | ||
| else { | ||
| mUi->generatorLabel->setText("Current directory exist."); |
There was a problem hiding this comment.
If the text is not intended to be shown to the user, please, state this clearly in the code. I recommend to use string literals like following
auto x = "*SOME TEMPORARY TEXT NOT FOR USER*"| mUi->bluetoothSettingsGroupBox->setVisible(robotModel->name() == mBluetoothRobotName); | ||
| if (QOperatingSystemVersion::currentType() == QOperatingSystemVersion::OSType::Unknown) { |
There was a problem hiding this comment.
Unknown means that QOperatingSystemVersion does not have a special literal name for a variant. I suggest using PlatformInfo::osType here
| QFile compile(dir.absolutePath() + "/compile.sh"); | ||
| mNxtToolsPresent = gnuarm.exists() && nexttool.exists() && nxtOSEK.exists() && compile.exists(); | ||
| #endif | ||
| switch(QOperatingSystemVersion::currentType()) { |
There was a problem hiding this comment.
I suggest using PlatformInfo::osType or introducing a companion function to that one to return some useful enumerable type.
| @@ -291,6 +303,10 @@ void NxtFlashTool::readNxtCompileData() | |||
| "If you sure in their validness contact developers")); | |||
| } else { | |||
| error(tr("Could not upload program. Make sure the robot is connected and ON")); | |||
| if (QOperatingSystemVersion::currentType() == QOperatingSystemVersion::OSType::Unknown) { | |||
There was a problem hiding this comment.
Unknown is not intended to check for Linux. See comments below
|
|
||
| switch(QOperatingSystemVersion::currentType()) { |
There was a problem hiding this comment.
Seems like QOperatingSystemVersion was not designed for this use-case, please, find additional info in my other comments.
| case QOperatingSystemVersion::OSType::Windows: { | ||
| auto pathToNxtTools = path().replace("\\", "/"); | ||
| auto line = path("compile.bat") | ||
| + " " + fileInfo.completeBaseName() |
There was a problem hiding this comment.
Probably, file names with spaces are handled incorrectly here after the single line is combined.
What?
I've added NXT support for GNU/Linux so this PR allows to compile and load programs via USB connection on Lego NXT.
Why?
Where's no NXT support for GNU/Linux, but it exist for Windows.
How?
I've changed structure of nxt-tools. There is no more division into win and linux directories. I use gcc-arm-none-eabi to compile C code on Linux same as on Windows. In order not to increase the volume of the installer, the user is given the opportunity to run the download-arm-none-eabi.sh script.
There are some changes in compile.sh and compile.bat to pass sysroot as argument and use sysroot as compile option. There are also added more setting to choose cross compiler path as shown on image below. I've added widget dirPicker to choose path.
Screenshots
Other info
The imagePicker widget exist, but it only allows you to select image path. To reduce code repetitions it would be a good idea to unify that widget and dirPicker.
Linked pull requests
PR to nxt-tools
PR to trik-help