Changeset View
Changeset View
Standalone View
Standalone View
src/core/atcore.cpp
Show First 20 Lines • Show All 882 Lines • ▼ Show 20 Line(s) | 877 | default: | |||
---|---|---|---|---|---|
883 | //QSerialPort::ParityError, Obsolete. Qt Docs "We strongly advise against using it in new code." | 883 | //QSerialPort::ParityError, Obsolete. Qt Docs "We strongly advise against using it in new code." | ||
884 | //QSerialPort::FramingError, Obsolete. Qt Docs "We strongly advise against using it in new code." | 884 | //QSerialPort::FramingError, Obsolete. Qt Docs "We strongly advise against using it in new code." | ||
885 | //QSerialPort::BreakConditionError, Obsolete. Qt Docs "We strongly advise against using it in new code." | 885 | //QSerialPort::BreakConditionError, Obsolete. Qt Docs "We strongly advise against using it in new code." | ||
886 | return; | 886 | return; | ||
887 | };//End of Switch | 887 | };//End of Switch | ||
888 | qCDebug(ATCORE_CORE) << "SerialError:" << errorString; | 888 | qCDebug(ATCORE_CORE) << "SerialError:" << errorString; | ||
889 | emit atcoreMessage(QStringLiteral("SerialError: %1").arg(errorString)); | 889 | emit atcoreMessage(QStringLiteral("SerialError: %1").arg(errorString)); | ||
890 | } | 890 | } | ||
891 | | ||||
892 | bool AtCore::toSd(const QString &fileName) | ||||
tcanabrava: I belive this function shouldn't return bool,
it should return enum class Reason { Success… | |||||
893 | { | ||||
894 | //Do a few sanity checks before attempting to write to SD. | ||||
895 | //Can't write an empty fileName. | ||||
896 | if (fileName.isEmpty()) { | ||||
897 | qCWarning(ATCORE_CORE) << "Sd Write Failed: Empty fileName."; | ||||
patrickelectric: This should be a warning or a critical. | |||||
898 | return false; | ||||
899 | } | ||||
900 | //Can not write if plugin doesn't have Sd Support. | ||||
901 | if (!d->firmwarePlugin->isSdSupported()) { | ||||
902 | qCDebug(ATCORE_CORE) << "Sd Write Failed: Firmware plugin does not support Sd functions."; | ||||
903 | return false; | ||||
904 | } | ||||
905 | | ||||
906 | //If the card is not mounted try to read the file list and mount it. | ||||
907 | if (!d->sdCardMounted) { | ||||
908 | getSDFileList(); | ||||
909 | if (!d->sdCardMounted) { | ||||
910 | qCDebug(ATCORE_CORE) << "Sd Write Failed unable to mount Sd Card"; | ||||
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… | |||||
911 | return false; | ||||
912 | } | ||||
913 | } | ||||
914 | // Be sure the printer is IDLE | ||||
915 | if (d->printerState == AtCore::DISCONNECTED) { | ||||
916 | qCDebug(ATCORE_CORE) << "No Device Connected."; | ||||
917 | return false; | ||||
tcanabrava: Const | |||||
patrickelectric: or not connected | |||||
918 | } else if (d->printerState != AtCore::IDLE) { | ||||
919 | qCDebug(ATCORE_CORE) << "Sd Write Failed: Device is Busy"; | ||||
920 | return false; | ||||
921 | } | ||||
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() | |||||
922 | | ||||
923 | //Process without thread. | ||||
924 | auto file = new QFile(fileName); | ||||
925 | if (!file->open(QFile::ReadOnly)) { | ||||
926 | qCDebug(ATCORE_CORE) << "Sd Write Failed unable to read file"; | ||||
927 | return false; | ||||
928 | } | ||||
929 | | ||||
930 | bool restoreTimer = d->tempTimer->isActive(); | ||||
931 | if (restoreTimer) { | ||||
patrickelectric: You need to check if file isReadable() | |||||
932 | d->tempTimer->stop(); | ||||
933 | } | ||||
patrickelectric: Need to check if was able to open it | |||||
934 | | ||||
935 | const auto fname = QFileInfo(fileName).fileName(); | ||||
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 | |||||
936 | const auto filesize = file->size(); | ||||
937 | auto txtStream = new QTextStream(file); | ||||
938 | auto stillSize = filesize; | ||||
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 | |||||
939 | int progress = 0; | ||||
patrickelectric: uint64_t | |||||
rizzitello: file->size returns type qint64 | |||||
940 | int lineNumber = -1; | ||||
patrickelectric: int | |||||
941 | int checksum = 0; | ||||
patrickelectric: uint64_t | |||||
rizzitello: fileSize of type qint64 so avoid warning. | |||||
942 | QString cline; | ||||
943 | | ||||
944 | setState(AtCore::BUSY); | ||||
945 | emit sdWriteChanged(true); | ||||
946 | pushCommand(GCode::toCommand(GCode::MCommands::M28, fname)); | ||||
947 | d->serial->waitForReadyRead(); | ||||
948 | | ||||
949 | bool waitForRead = true; | ||||
950 | while (waitForRead) { | ||||
951 | qCDebug(ATCORE_CORE) << " Waiting on return"; | ||||
952 | if (d->lastMessage.contains(QStringLiteral("Writing to file:").toLatin1())) { | ||||
953 | waitForRead = false; | ||||
954 | } | ||||
955 | d->serial->waitForReadyRead(); | ||||
956 | } | ||||
957 | | ||||
958 | while (!txtStream->atEnd()) { | ||||
patrickelectric: Category | |||||
959 | if (state() == AtCore::STOP || state() == AtCore::DISCONNECTED) { | ||||
960 | txtStream->setString(new QString()); | ||||
961 | cline = QString(); | ||||
962 | progress = 100; | ||||
963 | } else { | ||||
964 | cline = txtStream->readLine(); | ||||
965 | stillSize -= cline.size() + 1; //remove read chars | ||||
966 | progress = int((filesize - stillSize) * 100 / filesize); | ||||
967 | } | ||||
968 | if (!cline.startsWith(QStringLiteral(";"))) { | ||||
969 | lineNumber++; | ||||
970 | cline.prepend(QStringLiteral("N%1 ").arg(lineNumber)); | ||||
971 | checksum = firmwarePlugin()->checksum(cline); | ||||
972 | cline.insert(std::max(cline.indexOf(QStringLiteral(";")), cline.length()), QStringLiteral("*%1").arg(QString::number(checksum))); | ||||
973 | } | ||||
974 | if (!cline.isEmpty()) { | ||||
975 | d->ready = true; | ||||
976 | pushCommand(cline); | ||||
977 | d->serial->flush(); | ||||
978 | } | ||||
979 | emit sdWriteProgressChanged(progress); | ||||
980 | qCDebug(ATCORE_CORE) << "Progress:" << progress << "Current Line:" << cline; | ||||
981 | } | ||||
982 | | ||||
983 | pushCommand(GCode::toCommand(GCode::MCommands::M29, fname)); | ||||
984 | | ||||
985 | file->close(); | ||||
986 | emit sdWriteChanged(false); | ||||
987 | getSDFileList(); | ||||
988 | d->serial->flush(); | ||||
989 | setState(AtCore::IDLE); | ||||
990 | if (restoreTimer) { | ||||
991 | d->tempTimer->start(); | ||||
992 | } | ||||
993 | return true; | ||||
994 | } |
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?