Change wpa_supplicant copy #469
Conversation
recovery/mainwindow.cpp
Outdated
| _fixate = true; | ||
| } | ||
|
|
||
| copywpa(); |
There was a problem hiding this comment.
Should this have a capital W ?
There was a problem hiding this comment.
Yep. Don't know where it went - it's there in my original code. Odd.
recovery/mainwindow.cpp
Outdated
| qDebug() << "Copying /etc/wpa_supplicant.conf to /settings/wpa_supplicant.conf"; | ||
| QFile::copy("/etc/wpa_supplicant.conf", "/settings/wpa_supplicant.conf"); | ||
| } | ||
| qDebug() << "Copying user wpa_supplicant.conf to /settings/wpa_supplicant.conf"; |
There was a problem hiding this comment.
Two spaces between "Copying" and "user" ;)
recovery/mainwindow.cpp
Outdated
| QProcess::execute("mount -o remount,rw /settings"); | ||
| QProcess::execute("mount -o remount,rw /mnt"); | ||
|
|
||
| QFile::remove("/settings/wpa_supplicant.conf.bak"); |
There was a problem hiding this comment.
Perhaps it'd be worth making a little backup utility function which does something like (pseudocode):
bool backup(filename, ext="bak") {
backupname = filename + "." + ext;
if (exists(backupname)) {
remove(backupname);
}
return rename(filename, backupname);
}
recovery/mainwindow.cpp
Outdated
| QFile::copy("/mnt/wpa_supplicant.conf", "/settings/wpa_supplicant.conf"); | ||
| f.setPermissions( QFile::WriteUser | QFile::ReadGroup | QFile::ReadOther | QFile::ReadUser ); | ||
|
|
||
| /* rename the user file to avoid overwriting any manually set SSIDs */ |
There was a problem hiding this comment.
IMHO this comment is a bit confusing - is it essentially the same as saying "rename the user file to indicate that it has been copied (and prevent it being re-copied next time)" ?
Although I guess this breaks the "NOOBS prime directive" of not modifying any files on the RECOVERY partition. Although I've not been keeping up to date with NOOBS for a while, so I dunno if that prime-directive is still in place? ;)
There was a problem hiding this comment.
Yes. I was concentrating more on the consequences of it being copied a second time.
Dunno either, but I wasn't too concerned about that in PINN. Not sure if there's another way around it.
There was a problem hiding this comment.
I guess with having the same configuration file stored / copied / edited in two different places, there'll never be a "perfect" way of doing things, so it's probably one of those "pick the least worst" situations ;)
|
Any better? I've compiled it on another branch, but not tested. |
recovery/util.cpp
Outdated
| { | ||
|
|
||
| QString backupName = filename + "." + ext; | ||
| const char * backupFile = backupName.toUtf8().constData(); |
There was a problem hiding this comment.
Just out of curiosity, why the switch away from the Qt functions (QString, QFile::remove, QFile::rename, etc.) ? I think that might allow you to get rid of the ugly .toUtf8().constData() ?
There was a problem hiding this comment.
Sorry, I took your pseudo code too literally :) - I will revert.
recovery/util.h
Outdated
|
|
||
| QByteArray getFileContents(const QString &filename); | ||
| void putFileContents(const QString &filename, const QByteArray &data); | ||
| bool backup(QString filename, QString ext="bak"); |
There was a problem hiding this comment.
Perhaps this should be const QString &filename just like putFileContents uses? ;-)
| QFile::remove(backupName); | ||
| } | ||
| return rename(filename.toUtf8().constData(), backupFile); | ||
| return QFile::rename(filename,backupName); |
recovery/util.h
Outdated
| QByteArray getFileContents(const QString &filename); | ||
| void putFileContents(const QString &filename, const QByteArray &data); | ||
| bool backup(QString filename, QString ext="bak"); | ||
| bool backup(const QString &filename, const QString &ext="bak"); |
There was a problem hiding this comment.
Looking at the other functions declared here, perhaps backupFile would be a better name? :)
(sorry for being so nit-picky!)
Changed backup ->backupFile
Changed backup -> backupFile
Changed backup -> backupFile
|
Thanks |
3 participants
To fix #341 part 1.
Diff makes this look more complicated than it is.
Essentially the copy of wpa_supplicant.conf has been taken out of startNetwokring() into copywpa() which is then called ahead of startNetworking() regardless of silentinstall option.
Then copywpa() was modified to provide the necessary functionality.
(This PR is a copy/paste out of PINN into NOOBS)