Conversation
kasper201
left a comment
There was a problem hiding this comment.
Though you do use Dutch comments I'll approve cuz the rest looks good and we don't have time anymore :/
|
oepsie |
RoutesGenereren/mainwindow.cpp
Outdated
| // SerialPort + console instellingen | ||
| m_console->setEnabled(false); | ||
| QWidget* tab1 = m_console; | ||
| m_ui->tabWidget->addTab(tab1, "Developer mode"); |
There was a problem hiding this comment.
Dont show the user developer stuff
RoutesGenereren/mainwindow.cpp
Outdated
|
|
||
| void MainWindow::pushButton_2_clicked() | ||
| { | ||
| QByteArray dataToSend("help \r\n"); |
There was a problem hiding this comment.
This was just an example I added, where is the actual shell usage to let the user write/read to/from sd card?
|
|
||
| void optimizeRoute(vector<int>& route, vector<Point>& points) { | ||
| bool improved = true; | ||
| while (improved) { |
There was a problem hiding this comment.
while improved?
makes more sense to have the loop while not improved yet, this reads slightly odd
RoutesGenereren/routegenerator.cpp
Outdated
|
|
||
| void generate_routes() { | ||
| // definieer alle punten met microdegrees en kosten | ||
| allPoints = { |
There was a problem hiding this comment.
The user wanted to be able to add/remove/edit easily.
I don't see letting the user compile a program to adjust a location as easily.
Please adjust this to be loaded from a text file
There was a problem hiding this comment.
We only stated in the requirements that it must be possible to add trivia
There was a problem hiding this comment.
In Requirements v0.3.docx we have R10, and in our meeting on 13 February the customer said he'd like to be able to adjust questions and minigames.
Since questions are locked to locations, it is a side requirement that the customer must be able to add a location to be able to add a trivia question to it
TotallyOriginalUsername
left a comment
There was a problem hiding this comment.
There's also a main.cpp in the root folder, please move it away so a future group won't think it's the main file for the git repo.
There are also a lot of commits which could've been squashed
added error handling
RoutesGenereren/mainwindow.cpp
Outdated
| QMessageBox::warning(this, tr("Warning"), message); | ||
| } | ||
|
|
||
| void MainWindow::sendJsonFileToDevice(const QString& filePath) |
There was a problem hiding this comment.
This is the equivalent to echoing a string in your terminal and doesn't save it to the text file on the device
RoutesGenereren/mainwindow.cpp
Outdated
| m_serial->write(footer); | ||
| m_serial->waitForBytesWritten(100); | ||
|
|
||
| QMessageBox::information(this, tr("Success"), tr("File %1 sent to device").arg(filePath)); |
There was a problem hiding this comment.
Where is the error handling?
RoutesGenereren/mainwindow.h
Outdated
| void showStatusMessage(const QString &message); | ||
| void showWriteError(const QString &message); | ||
| void drawRoutes(); | ||
| void sendJsonFileToDevice(const QString& filePath); |
There was a problem hiding this comment.
There is no receiving scores, nor sending trivia?
RoutesGenereren/mainwindow.cpp
Outdated
|
|
||
| // Open serial port | ||
| QSerialPort port(portName, this); | ||
| port.setBaudRate(QSerialPort::Baud9600); |
There was a problem hiding this comment.
this looks like it overwrites settings? is that true?
There was a problem hiding this comment.
relevant as the device default is 115200, so this is misleading, and likely to cause issues
RoutesGenereren/mainwindow.cpp
Outdated
| } | ||
|
|
||
| // Send JSON-route | ||
| port.write(jsonData); |
There was a problem hiding this comment.
i have a bad feeling about this if this tries to achieve what i think it tries to achieve.
i doubt files will be saved
No description provided.