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

Implement uart permission check in a more distro independent way #20

Open
wants to merge 1 commit into
base: OpenFIRE-dev
Choose a base branch
from
Open

Implement uart permission check in a more distro independent way #20

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 31 additions & 23 deletions guiwindow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@
#include <QDesktopServices>
#include <QUrl>

#if !defined(Q_OS_MAC) && !defined(Q_OS_WIN)
#include <unistd.h>
#include <sys/stat.h>
#endif



// Currently loaded board object
boardInfo_s board;

Expand Down Expand Up @@ -121,6 +128,23 @@ void guiWindow::PortsSearch()
if(!usbName.length()) {
PopupWindow("No OpenFIRE devices detected!", "Is the microcontroller board currently running OpenFIRE and is currently plugged in? Make sure it's connected and recognized by the PC.\n\nThis app will now close.", "ERROR", 4);
exit(1);
}else{
Copy link
Member

Choose a reason for hiding this comment

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

Weird to have an unresolved if branch that isn't inside the Unix conditional block? This should be bumped down to be part of if(getuid()) (and the closing brace should be outside of it too). Also consistent spacing.

#if !defined(Q_OS_MAC) && !defined(Q_OS_WIN)
if(getuid()) {
auto deviceName = usbName.first();
Copy link
Member

Choose a reason for hiding this comment

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

Question: does this actually work if we do not have any compatible devices plugged in? I was under the assumption that users outside of dialout don't get any devices (and we've already pruned out any system ttySX virt terminals at this point).

It would be ideal to inform the user first that they should have an ideal working user environment - which is how the original implem worked, then move to available device checks. Perhaps move this block to above/below checking the length of our available devices list?

QFileInfo fi(deviceName);
auto neededGroup = fi.group();
auto neededGid = fi.groupId();
if(!group_member(neededGid)) {
PopupWindow("User doesn't have serial permissions!",
QString("Currently, your user is not allowed to have access to serial devices.\n\nTo add yourself to the right group, run this command in a terminal and then re-login to your session: \n\nsudo usermod -aG %2 %1").arg(qEnvironmentVariable("USER")).arg(neededGroup), "Permission error", 2);
exit(0);
}
} else {
PopupWindow("Running as root is not allowed!", "Please run the OpenFIRE app as a normal user.", "ERROR", 4);
exit(2);
}
#endif
}
}
}
Expand All @@ -131,22 +155,6 @@ guiWindow::guiWindow(QWidget *parent)
{
ui->setupUi(this);

#if !defined(Q_OS_MAC) && !defined(Q_OS_WIN)
if(qEnvironmentVariable("USER") != "root") {
QProcess *externalProg = new QProcess;
QStringList args;
externalProg->start("/usr/bin/groups", args);
externalProg->waitForFinished();
if(!externalProg->readAllStandardOutput().contains("dialout")) {
PopupWindow("User doesn't have serial permissions!", QString("Currently, your user is not allowed to have access to serial devices.\n\nTo add yourself to the right group, run this command in a terminal and then re-login to your session: \n\nsudo usermod -aG dialout %1").arg(qEnvironmentVariable("USER")), "Permission error", 2);
exit(0);
}
} else {
PopupWindow("Running as root is not allowed!", "Please run the OpenFIRE app as a normal user.", "ERROR", 4);
exit(2);
}
#endif

connect(&serialPort, &QSerialPort::readyRead, this, &guiWindow::serialPort_readyRead);

// just to be sure, init the inputsMap hashes
Expand Down Expand Up @@ -2204,15 +2212,15 @@ void guiWindow::serialPort_readyRead()
{
if(!serialActive) {
while(!serialPort.atEnd()) {
QString idleBuffer = serialPort.readLine();
QString idleBuffer = serialPort.readLine().trimmed();
if(idleBuffer.contains("Pressed:")) {
uint8_t button = idleBuffer.trimmed().rightRef(2).toInt();
uint8_t button = idleBuffer.right(2).toInt();
testLabel[button-1]->setText(QString("<font color=#FF0000>%1</font>").arg(valuesNameList[button]));
} else if(idleBuffer.contains("Released:")) {
uint8_t button = idleBuffer.trimmed().rightRef(2).toInt();
uint8_t button = idleBuffer.right(2).toInt();
testLabel[button-1]->setText(valuesNameList[button]);
} else if(idleBuffer.contains("Temperature:")) {
uint8_t temp = idleBuffer.trimmed().rightRef(2).toInt();
uint8_t temp = idleBuffer.right(2).toInt();
if(temp > tempShutoff) {
testLabel[14]->setText(QString("<font color=#FF0000>Temp: %1°C</font>").arg(temp));
} else if(temp > tempWarning) {
Expand All @@ -2221,7 +2229,7 @@ void guiWindow::serialPort_readyRead()
testLabel[14]->setText(QString("<font color=#11D00A>Temp: %1°C</font>").arg(temp));
}
} else if(idleBuffer.contains("Analog:")) {
uint8_t analogDir = idleBuffer.trimmed().rightRef(1).toInt();
uint8_t analogDir = idleBuffer.right(1).toInt();
if(analogDir) {
switch(analogDir) {
case 1: testLabel[15]->setText("<font color=#FF0000>Analog 🡹</font>"); break;
Expand All @@ -2238,14 +2246,14 @@ void guiWindow::serialPort_readyRead()
}
// no idea here lol
} else if(idleBuffer.contains("Profile: ")) {
uint8_t selection = idleBuffer.trimmed().rightRef(1).toInt();
uint8_t selection = idleBuffer.right(1).toInt();
if(selection != board.selectedProfile) {
board.selectedProfile = selection;
selectedProfile[selection]->setChecked(true);
}
DiffUpdate();
} else if(idleBuffer.contains("UpdatedProf: ")) {
uint8_t selection = idleBuffer.trimmed().rightRef(1).toInt();
uint8_t selection = idleBuffer.right(1).toInt();
if(selection != board.selectedProfile) {
selectedProfile[selection]->setChecked(true);
}
Expand Down
Loading