Changeset View
Changeset View
Standalone View
Standalone View
src/core/atcore.cpp
Show First 20 Lines • Show All 774 Lines • ▼ Show 20 Line(s) | |||||
775 | void AtCore::sdCardPrintStatus() | 775 | void AtCore::sdCardPrintStatus() | ||
776 | { | 776 | { | ||
777 | //One request for the Sd Job status in the queue at a time. | 777 | //One request for the Sd Job status in the queue at a time. | ||
778 | if (d->commandQueue.contains(GCode::toCommand(GCode::M27))) { | 778 | if (d->commandQueue.contains(GCode::toCommand(GCode::M27))) { | ||
779 | return; | 779 | return; | ||
780 | } | 780 | } | ||
781 | pushCommand(GCode::toCommand(GCode::M27)); | 781 | pushCommand(GCode::toCommand(GCode::M27)); | ||
782 | } | 782 | } | ||
783 | | ||||
784 | bool AtCore::writeFiletoSd(const QString &fileName) | ||||
tcanabrava: I belive this function shouldn't return bool,
it should return enum class Reason { Success… | |||||
785 | { | ||||
786 | //Do a few sanity checks before attempting to write to SD. | ||||
787 | | ||||
788 | //Can't write an empty fileName. | ||||
789 | if (fileName.isEmpty()) { | ||||
patrickelectric: This should be a warning or a critical. | |||||
790 | return false; | ||||
791 | } | ||||
792 | //Can not write if plugin doesn't have Sd Support. | ||||
793 | if (!d->firmwarePlugin->isSdSupported()) { | ||||
794 | return false; | ||||
795 | } | ||||
796 | | ||||
797 | //If the card is not mounted try to read the file list and mount it. | ||||
798 | if (!d->sdCardMounted) { | ||||
799 | getSDFileList(); | ||||
800 | if (!d->sdCardMounted) { | ||||
801 | return false; | ||||
802 | } | ||||
It’s good to say “could not mount se card” somehow, instead of just returning false. Same for the other checks tcanabrava: It’s good to say “could not mount se card” somehow, instead of just returning false. Same for… | |||||
803 | } | ||||
804 | // Be sure the printer is IDLE | ||||
805 | if (d->printerState != AtCore::IDLE) { | ||||
806 | return false; | ||||
807 | } | ||||
808 | | ||||
809 | QString fname = fileName.mid(fileName.lastIndexOf(QStringLiteral("/")) + 1, fileName.length()); | ||||
tcanabrava: Const | |||||
patrickelectric: or not connected | |||||
810 | QFile *file = new QFile(fileName); | ||||
811 | file->open(QFile::ReadOnly); | ||||
812 | QTextStream *txtStream = new QTextStream(file); | ||||
813 | | ||||
patrickelectric: QUrl::filename | |||||
fileName is a QString and we have no other QUrl in this object. if we add that we should do so in another patch. rizzitello: fileName is a QString and we have no other QUrl in this object. if we add that we should do so… | |||||
patrickelectric: QFileInfo.fileName() | |||||
814 | setState(AtCore::BUSY); | ||||
815 | pushCommand(GCode::toCommand(GCode::M28, fname)); | ||||
816 | | ||||
817 | while (!txtStream->atEnd()) { | ||||
818 | pushCommand(txtStream->readLine()); | ||||
819 | } | ||||
820 | | ||||
821 | file->close(); | ||||
822 | pushCommand(GCode::toCommand(GCode::M29, fname)); | ||||
823 | setState(AtCore::IDLE); | ||||
patrickelectric: You need to check if file isReadable() | |||||
824 | getSDFileList(); | ||||
825 | return true; | ||||
patrickelectric: Need to check if was able to open it | |||||
826 | } | ||||
This needs to be adjusted to return true after the thread has finished rizzitello: This needs to be adjusted to return true after the thread has finished | |||||
How do we poke the thread to see it’s state? I’d store the variable in the class or something tcanabrava: How do we poke the thread to see it’s state? I’d store the variable in the class or something | |||||
patrickelectric: Category | |||||
patrickelectric: uint64_t | |||||
rizzitello: file->size returns type qint64 | |||||
patrickelectric: uint64_t | |||||
rizzitello: fileSize of type qint64 so avoid warning. | |||||
patrickelectric: int |
I belive this function shouldn't return bool,
it should return enum class Reason { Success , EmptyFilename, FirmwareNotSupported, DeviceBusy } so the interface (be it qml, desktop or any other interface created for atcore can display to the user an message if something wrong occoured. This message can even be returned by atCore.lastError() or something. what you think?