From e0a745d241822193f877a801151ff3f12a4967f9 Mon Sep 17 00:00:00 2001 From: Andre Hartmann Date: Sat, 25 Nov 2017 19:29:12 +0100 Subject: Canbusutil: General cleanup and simplification * Order includes and use forward-declarations * Use member initialization and nullptr * Transfrom last connects to Qt5 style * Translate output messages * Name slots better * Discard frames with invalid frame ID or payload size * Simplified some error handling and error outputting Change-Id: Id4d4329afe909ba932e91ad1deeae411806fd77f Reviewed-by: Alex Blasche Reviewed-by: Rolf Eike Beer --- src/tools/canbusutil/canbusutil.cpp | 96 ++++++++++++++++++++----------------- src/tools/canbusutil/canbusutil.h | 16 +++---- src/tools/canbusutil/main.cpp | 26 +++++----- src/tools/canbusutil/readtask.cpp | 19 ++++---- src/tools/canbusutil/readtask.h | 8 ++-- 5 files changed, 84 insertions(+), 81 deletions(-) (limited to 'src/tools') diff --git a/src/tools/canbusutil/canbusutil.cpp b/src/tools/canbusutil/canbusutil.cpp index c553c52..38433bb 100644 --- a/src/tools/canbusutil/canbusutil.cpp +++ b/src/tools/canbusutil/canbusutil.cpp @@ -36,8 +36,11 @@ #include "canbusutil.h" -CanBusUtil::CanBusUtil(QTextStream &output, QCoreApplication &app, QObject *parent) - : QObject(parent), +#include +#include + +CanBusUtil::CanBusUtil(QTextStream &output, QCoreApplication &app, QObject *parent) : + QObject(parent), m_canBus(QCanBus::instance()), m_output(output), m_app(app), @@ -58,7 +61,7 @@ void CanBusUtil::setShowFdFlags(bool showFdFlags) bool CanBusUtil::start(const QString &pluginName, const QString &deviceName, const QString &data) { if (!m_canBus) { - m_output << "Unable to create QCanBus" << endl; + m_output << tr("Error: Cannot create QCanBus.") << endl; return false; } @@ -73,29 +76,41 @@ bool CanBusUtil::start(const QString &pluginName, const QString &deviceName, con if (m_listening) { if (m_readTask->isShowFdFlags()) m_canDevice->setConfigurationParameter(QCanBusDevice::CanFdKey, true); - connect(m_canDevice.data(), &QCanBusDevice::framesReceived, m_readTask, &ReadTask::checkMessages); + connect(m_canDevice.data(), &QCanBusDevice::framesReceived, + m_readTask, &ReadTask::handleFrames); } else { if (!sendData()) return false; - QTimer::singleShot(0, &m_app, SLOT(quit())); + QTimer::singleShot(0, &m_app, QCoreApplication::quit); } return true; } -void CanBusUtil::printPlugins() +int CanBusUtil::printPlugins() { + if (!m_canBus) { + m_output << tr("Error: Cannot create QCanBus.") << endl; + return 1; + } + const QStringList plugins = m_canBus->plugins(); - for (int i = 0; i < plugins.size(); i++) - m_output << plugins.at(i) << endl; + for (const QString &plugin : plugins) + m_output << plugin << endl; + return 0; } int CanBusUtil::printDevices(const QString &pluginName) { + if (!m_canBus) { + m_output << tr("Error: Cannot create QCanBus.") << endl; + return 1; + } + QString errorMessage; const QList devices = m_canBus->availableDevices(pluginName, &errorMessage); if (!errorMessage.isEmpty()) { - m_output << "Error: " << errorMessage << endl; + m_output << tr("Error gathering available devices: '%1'").arg(errorMessage) << endl; return 1; } @@ -108,12 +123,12 @@ bool CanBusUtil::parseDataField(quint32 &id, QString &payload) { int hashMarkPos = m_data.indexOf('#'); if (hashMarkPos < 0) { - m_output << "Data field invalid: No hash mark found!" << endl; + m_output << tr("Data field invalid: No hash mark found!") << endl; return false; } id = m_data.leftRef(hashMarkPos).toUInt(nullptr, 16); - payload = m_data.right(m_data.length() - hashMarkPos - 1); + payload = m_data.right(m_data.size() - hashMarkPos - 1); return true; } @@ -121,37 +136,30 @@ bool CanBusUtil::parseDataField(quint32 &id, QString &payload) bool CanBusUtil::setFrameFromPayload(QString payload, QCanBusFrame *frame) { if (!payload.isEmpty() && payload.at(0).toUpper() == 'R') { - bool validPayloadLength = false; - frame->setFrameType(QCanBusFrame::RemoteRequestFrame); - if (payload.size() == 1) { // payload = "R" - validPayloadLength = true; - } else if (payload.size() > 1) { // payload = "R8" - payload = payload.mid(1); - int rtrFrameLength = payload.toInt(&validPayloadLength); - - if (validPayloadLength && rtrFrameLength >= 0 && rtrFrameLength <= 8) { - frame->setPayload(QByteArray(rtrFrameLength, 0)); - } else if (validPayloadLength) { - m_output << "The length must be between 0 and 8 (including)." << endl; - validPayloadLength = false; - } - } + if (payload.size() == 1) // payload = "R" + return true; - if (!validPayloadLength) { - m_output << "Data field invalid: RTR data frame length not specified/valid." << endl; + bool ok = false; + int rtrFrameLength = payload.midRef(1).toInt(&ok); + if (ok && rtrFrameLength >= 0 && rtrFrameLength <= 8) { // payload = "R8" + frame->setPayload(QByteArray(rtrFrameLength, 0)); + return true; } - return validPayloadLength; - } else if (!payload.isEmpty() && payload.at(0) == '#') { + m_output << tr("Error: RTR frame length must be between 0 and 8 (including).") << endl; + return false; + } + + if (!payload.isEmpty() && payload.at(0) == '#') { frame->setFlexibleDataRateFormat(true); - payload = payload.mid(1); + payload.remove(0, 1); } const QRegularExpression re(QStringLiteral("^[0-9A-Fa-f]*$")); if (!re.match(payload).hasMatch()) { - m_output << "Data field invalid: Only hex numbers allowed." << endl; + m_output << tr("Data field invalid: Only hex numbers allowed.") << endl; return false; } @@ -163,7 +171,7 @@ bool CanBusUtil::setFrameFromPayload(QString payload, QCanBusFrame *frame) frame->setErrorStateIndicator(flags & ErrorStateIndicatorFlag); payload.remove(0, 1); } else { - m_output << "Data field invalid: Size is not multiple of two." << endl; + m_output << tr("Data field invalid: Size is not multiple of two.") << endl; return false; } } @@ -172,8 +180,8 @@ bool CanBusUtil::setFrameFromPayload(QString payload, QCanBusFrame *frame) const int maxSize = frame->hasFlexibleDataRateFormat() ? 64 : 8; if (bytes.size() > maxSize) { - m_output << "Warning: Truncating payload at max. size of " << maxSize << " bytes." << endl; - bytes.truncate(maxSize); + m_output << tr("Data field invalid: Size is longer than %1 bytes.").arg(maxSize) << endl; + return false; } frame->setPayload(bytes); @@ -184,20 +192,18 @@ bool CanBusUtil::setFrameFromPayload(QString payload, QCanBusFrame *frame) bool CanBusUtil::connectCanDevice() { if (!m_canBus->plugins().contains(m_pluginName)) { - m_output << "Could not find suitable plugin." << endl; - m_output << "Available plugins:" << endl; - printPlugins(); + m_output << tr("Cannot find CAN bus plugin '%1'.").arg(m_pluginName) << endl; return false; } m_canDevice.reset(m_canBus->createDevice(m_pluginName, m_deviceName)); if (!m_canDevice) { - m_output << "Unable to create QCanBusDevice with device name: " << m_deviceName << endl; + m_output << tr("Cannot create CAN bus device: '%1'").arg(m_deviceName) << endl; return false; } - connect(m_canDevice.data(), &QCanBusDevice::errorOccurred, m_readTask, &ReadTask::receiveError); + connect(m_canDevice.data(), &QCanBusDevice::errorOccurred, m_readTask, &ReadTask::handleError); if (!m_canDevice->connectDevice()) { - m_output << "Unable to connect QCanBusDevice with device name: " << m_deviceName << endl; + m_output << tr("Cannot create CAN bus device: '%1'").arg(m_deviceName) << endl; return false; } @@ -210,15 +216,15 @@ bool CanBusUtil::sendData() QString payload; QCanBusFrame frame; - if (parseDataField(id, payload) == false) + if (!parseDataField(id, payload)) return false; - if (setFrameFromPayload(payload, &frame) == false) + if (!setFrameFromPayload(payload, &frame)) return false; if (id > 0x1FFFFFFF) { // 29 bits - id = 0x1FFFFFFF; - m_output << "Warning! Id does not fit into Extended Frame Format, setting id to: " << id << endl; + m_output << tr("Cannot send invalid frame ID: '%1'").arg(id, 0, 16) << endl; + return false; } frame.setFrameId(id); diff --git a/src/tools/canbusutil/canbusutil.h b/src/tools/canbusutil/canbusutil.h index 356b3df..e6432a3 100644 --- a/src/tools/canbusutil/canbusutil.h +++ b/src/tools/canbusutil/canbusutil.h @@ -37,16 +37,16 @@ #ifndef CANBUSUTIL_H #define CANBUSUTIL_H +#include "readtask.h" + #include -#include -#include #include -#include "readtask.h" - QT_BEGIN_NAMESPACE class QCanBusFrame; +class QCoreApplication; +class QTextStream; QT_END_NAMESPACE @@ -59,7 +59,7 @@ public: void setShowTimeStamp(bool showTimeStamp); void setShowFdFlags(bool showFdFlags); bool start(const QString &pluginName, const QString &deviceName, const QString &data = QString()); - void printPlugins(); + int printPlugins(); int printDevices(const QString &pluginName); private: @@ -69,15 +69,15 @@ private: bool sendData(); private: - QCanBus *m_canBus; + QCanBus *m_canBus = nullptr; QTextStream &m_output; QCoreApplication &m_app; - bool m_listening; + bool m_listening = false; QString m_pluginName; QString m_deviceName; QString m_data; QScopedPointer m_canDevice; - ReadTask *m_readTask; + ReadTask *m_readTask = nullptr; }; #endif // CANBUSUTIL_H diff --git a/src/tools/canbusutil/main.cpp b/src/tools/canbusutil/main.cpp index 1304f22..6744f19 100644 --- a/src/tools/canbusutil/main.cpp +++ b/src/tools/canbusutil/main.cpp @@ -34,6 +34,9 @@ ** ****************************************************************************/ +#include "canbusutil.h" +#include "sigtermhandler.h" + #include #include #include @@ -41,15 +44,10 @@ #include -#include "canbusutil.h" -#include "sigtermhandler.h" - -#define PROGRAMNAME "canbusutil" - int main(int argc, char *argv[]) { QCoreApplication app(argc, argv); - QCoreApplication::setApplicationName(QStringLiteral(PROGRAMNAME)); + QCoreApplication::setApplicationName(QStringLiteral("canbusutil")); QCoreApplication::setApplicationVersion(QStringLiteral(QT_VERSION_STR)); QScopedPointer s(SigTermHandler::instance()); @@ -106,10 +104,8 @@ int main(int argc, char *argv[]) parser.process(app); - if (parser.isSet(listOption)) { - util.printPlugins(); - return 0; - } + if (parser.isSet(listOption)) + return util.printPlugins(); QString data; const QStringList args = parser.positionalArguments(); @@ -117,16 +113,16 @@ int main(int argc, char *argv[]) util.setShowTimeStamp(parser.isSet(showTimeStampOption)); util.setShowFdFlags(parser.isSet(showFdFlagsOption)); } else if (args.size() == 3) { - data = args[2]; + data = args.at(2); } else if (args.size() == 1 && parser.isSet(listDevicesOption)) { - return util.printDevices(args[0]); + return util.printDevices(args.at(0)); } else if (args.size() != 2) { - fprintf(stderr, "Invalid number of arguments (%d given).\n\n%s", - args.size(), qPrintable(parser.helpText())); + output << CanBusUtil::tr("Invalid number of arguments (%1 given).").arg(args.size()); + output << endl << endl << parser.helpText(); return 1; } - if (!util.start(args[0], args[1], data)) + if (!util.start(args.at(0), args.at(1), data)) return -1; return app.exec(); diff --git a/src/tools/canbusutil/readtask.cpp b/src/tools/canbusutil/readtask.cpp index fb159b7..2bde509 100644 --- a/src/tools/canbusutil/readtask.cpp +++ b/src/tools/canbusutil/readtask.cpp @@ -36,9 +36,9 @@ #include "readtask.h" -ReadTask::ReadTask(QTextStream &output, QObject *parent) - : QObject(parent), - output(output) { } +ReadTask::ReadTask(QTextStream &output, QObject *parent) : + QObject(parent), + m_output(output) { } void ReadTask::setShowTimeStamp(bool showTimeStamp) { @@ -55,10 +55,10 @@ void ReadTask::setShowFdFlags(bool showFlags) m_showFdFlags = showFlags; } -void ReadTask::checkMessages() { +void ReadTask::handleFrames() { auto canDevice = qobject_cast(QObject::sender()); if (canDevice == nullptr) { - qWarning("ReadTask::checkMessages: Unknown sender"); + qWarning("ReadTask::handleFrames: Unknown sender."); return; } @@ -87,16 +87,17 @@ void ReadTask::checkMessages() { else view += frame.toString(); - output << view << endl; + m_output << view << endl; } } -void ReadTask::receiveError(QCanBusDevice::CanBusError /*error*/) { +void ReadTask::handleError(QCanBusDevice::CanBusError /*error*/) +{ auto canDevice = qobject_cast(QObject::sender()); if (canDevice == nullptr) { - qWarning("ReadTask::receiveError: Unknown sender"); + qWarning("ReadTask::handleError: Unknown sender."); return; } - output << "Read error: " << canDevice->errorString() << endl; + m_output << tr("Read error: '%1'").arg(canDevice->errorString()) << endl; } diff --git a/src/tools/canbusutil/readtask.h b/src/tools/canbusutil/readtask.h index e00440a..a866d3d 100644 --- a/src/tools/canbusutil/readtask.h +++ b/src/tools/canbusutil/readtask.h @@ -45,17 +45,17 @@ class ReadTask : public QObject { Q_OBJECT public: - explicit ReadTask(QTextStream &output, QObject *parent = nullptr); + explicit ReadTask(QTextStream &m_output, QObject *parent = nullptr); void setShowTimeStamp(bool showStamp); bool isShowFdFlags() const; void setShowFdFlags(bool isShowFdFlags); public slots: - void checkMessages(); - void receiveError(QCanBusDevice::CanBusError /*error*/); + void handleFrames(); + void handleError(QCanBusDevice::CanBusError /*error*/); private: - QTextStream &output; + QTextStream &m_output; bool m_showTimeStamp = false; bool m_showFdFlags = false; }; -- cgit v1.2.3