Serial communication
-
@jsulm Do you mean about this notation:
m_responseStatus = (Response::Status)m_request.mid(2,1).toInt(nullptr, 16);
?
@Damian7546 Something like this. But better don't use c-style casts:
m_responseStatus = static_cast<Response::Status>(m_request.mid(2,1).toInt(nullptr, 16));
-
@Damian7546 Something like this. But better don't use c-style casts:
m_responseStatus = static_cast<Response::Status>(m_request.mid(2,1).toInt(nullptr, 16));
@jsulm Thank you.
On base above proposition, I prepared conception synchronous serial communication (without blocking) with my device using state machine.
- Firstly I changed old Qt4 string based connections,
- Secondly I prefer use
switch
instruction insteadQStateMachine
This is only skeleton, without any tests . I would like to know yours opinion, What do you think about below solution:
#include "mydevice.h" MyDevice::MyDevice(QObject *parent) : QObject{parent} { serial = new QSerialPort(); serial->setPortName("COM9"); serial->setBaudRate(QSerialPort::Baud9600); serial->setDataBits(QSerialPort::Data8); serial->setParity(QSerialPort::EvenParity); serial->open(QIODevice::ReadWrite); connect(serial, &QSerialPort::readyRead, this, &MyDevice::serialRecive); connect(&timerResponse, &QTimer::timeout, this, [&](){ m_timeoutResponse = true; timerResponse.stop(); qDebug()<<"Timeout response";} ); connect(&timerState, &QTimer::timeout, this, &MyDevice::stateMachine); if(!serial->isOpen()){ qInfo() << "Serial port status: " << serial->isOpen(); }else{ //run state machine! m_state = State::state::STATUS_REQUEST; timerState.start(100); } } void MyDevice::serialRecive() { m_request.append(serial->readAll()); while(!messageComplete(m_request) && !m_timeoutResponse) { if(!serial->waitForReadyRead(20)){ qDebug()<<"Incomplete response"; m_state = State::state::STATUS_REQUEST; return; } m_request.append(serial->readAll()); } bool crcValid=validateResponse(m_request); if(!crcValid){ qDebug()<<"Crc error"; m_state = State::state::STATUS_REQUEST; return; } if(m_timeoutResponse) { qDebug()<<"Timeout response"; m_state = State::state::STATUS_REQUEST; return; } timerResponse.stop(); qDebug()<<"Correct response: " << m_request; } void MyDevice::stateMachine() { m_responseStatus = static_cast<Response::Status>(m_request.mid(2,1).toInt(nullptr, 16)); switch (m_state) { case State::state::STATUS_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::POLL,m_data)); m_state = State::state::STATUS_RESPONSE; break; case State::state::STATUS_RESPONSE: if(m_responseStatus == Response::Status::POWER_UP) m_state = State::state::VERSION_REQUEST; if(m_responseStatus == Response::Status::POWER_UP_WITH_BILL_IN_ACCEPTOR) m_state = State::state::RESET_REQUEST; if(m_responseStatus == Response::Status::POWER_UP_WITH_BILL_IN_STACKER) m_state = State::state::RESET_REQUEST; if(m_responseStatus == Response::Status::INITIALIZE) m_state = State::state::EN_DIS_REQUEST; if(m_responseStatus == Response::Status::ENABLE) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::ACCEPTING) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::ESCROW) m_state = State::state::STACK1_REQUEST; if(m_responseStatus == Response::Status::STACKING) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::STACKED) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::VALID) m_state = State::state::ACK_REQUEST; if(m_responseStatus == Response::Status::REJECTING) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::JAM_IN_ACCEPTOR) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::VERSION_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::GET_VERSION,m_data)); m_state = State::state::VERSION_RESPONSE; break; case State::state::VERSION_RESPONSE: if(m_responseStatus == Response::Status::STAT_VERSION) m_state = State::state::RESET_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::RESET_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::RESET,m_data)); m_state = State::state::RESET_RESPONSE; break; case State::state::RESET_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::EN_DIS_REQUEST: timerResponse.start(m_timeout); clearBuffers(); m_data = QByteArray("\xff\xff",2); serial->write(createMessage(Commands::deviceCommand::ENABLE,m_data)); m_state = State::state::EN_DIS_RESPONSE; break; case State::state::EN_DIS_RESPONSE: if(m_responseStatus == Response::Status::STAT_EN_DIS) m_state = State::state::SECURITY_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::SECURITY_REQUEST: timerResponse.start(m_timeout); clearBuffers(); m_data = QByteArray("\xff\xff",2); serial->write(createMessage(Commands::deviceCommand::SECURITY,m_data)); m_state = State::state::SECURITY_REQUEST; break; case State::state::SECURITY_RESPONSE: if(m_responseStatus == Response::Status::CMD_ENABLE_SECURITY) m_state = State::state::OPTIONAL_FUN_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::OPTIONAL_FUN_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::OPTIONAL,m_data)); m_state = State::state::OPTIONAL_FUN_RESPONSE; break; case State::state::OPTIONAL_FUN_RESPONSE: if(m_responseStatus == Response::Status::CMD_OPTIONAL) m_state = State::state::INHIBIT_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::INHIBIT_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::INHIBIT,m_data)); m_state = State::state::INHIBIT_RESPONSE; break; case State::state::INHIBIT_RESPONSE: if(m_responseStatus == Response::Status::CMD_INHIBIT) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::STACK1_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::STACK1,m_data)); m_state = State::state::STACK1_RESPONSE; break; case State::state::STACK1_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::STACK2_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::STACK2,m_data)); m_state = State::state::STACK2_RESPONSE; break; case State::state::STACK2_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::ACK_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::ACK,m_data)); m_state = State::state::STATUS_REQUEST; break; case State::state::RETURN_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::RETURN,m_data)); m_state = State::state::RETURN_RESPONSE; break; case State::state::RETURN_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; default: break; } } void MyDevice::clearBuffers() { m_request.clear(); m_data.clear(); } bool MyDevice::messageComplete(const QByteArray &data) const { if(data.size()>=3){ quint8 length=(unsigned char)data[1]; if(length==data.length()) return true; } return false; } bool MyDevice::validateResponse(QByteArray data) { if(data.size()<3) return false; QByteArray crc=data.right(2); std::reverse(crc.begin(),crc.end()); bool ok=false; quint16 messageCRC=crc.toHex().toUInt(&ok,16); if(!ok) return false; data.chop(2); return (messageCRC==crc16(data)); } QByteArray MyDevice::createMessage(const Commands::deviceCommand &cmd, const QByteArray &data) { QByteArray message; message.append(0xFC); if(data.count() > 0) { message.append(0x05+data.count()); } else{ message.append(0x05); } message.append((char)cmd); if(data.count() > 0) { for (int i= 0 ; i< data.count() ; i++) message.append(data[i]); } quint16 crc=crc16(message); QByteArray crcData=QByteArray((char *)&crc,2); message.append(crcData); return message; } quint16 MyDevice::crc16(const QByteArray &data) { unsigned int CRC; unsigned char j; CRC = 0; for(int i=0; i < data.size(); i++) { CRC ^= (unsigned char)data[i]; for(j=0; j < 8; j++) { if(CRC & 0x0001) {CRC >>= 1; CRC ^= POLYNOMIAL;} else CRC >>= 1; } } return CRC; }
-
@jsulm Thank you.
On base above proposition, I prepared conception synchronous serial communication (without blocking) with my device using state machine.
- Firstly I changed old Qt4 string based connections,
- Secondly I prefer use
switch
instruction insteadQStateMachine
This is only skeleton, without any tests . I would like to know yours opinion, What do you think about below solution:
#include "mydevice.h" MyDevice::MyDevice(QObject *parent) : QObject{parent} { serial = new QSerialPort(); serial->setPortName("COM9"); serial->setBaudRate(QSerialPort::Baud9600); serial->setDataBits(QSerialPort::Data8); serial->setParity(QSerialPort::EvenParity); serial->open(QIODevice::ReadWrite); connect(serial, &QSerialPort::readyRead, this, &MyDevice::serialRecive); connect(&timerResponse, &QTimer::timeout, this, [&](){ m_timeoutResponse = true; timerResponse.stop(); qDebug()<<"Timeout response";} ); connect(&timerState, &QTimer::timeout, this, &MyDevice::stateMachine); if(!serial->isOpen()){ qInfo() << "Serial port status: " << serial->isOpen(); }else{ //run state machine! m_state = State::state::STATUS_REQUEST; timerState.start(100); } } void MyDevice::serialRecive() { m_request.append(serial->readAll()); while(!messageComplete(m_request) && !m_timeoutResponse) { if(!serial->waitForReadyRead(20)){ qDebug()<<"Incomplete response"; m_state = State::state::STATUS_REQUEST; return; } m_request.append(serial->readAll()); } bool crcValid=validateResponse(m_request); if(!crcValid){ qDebug()<<"Crc error"; m_state = State::state::STATUS_REQUEST; return; } if(m_timeoutResponse) { qDebug()<<"Timeout response"; m_state = State::state::STATUS_REQUEST; return; } timerResponse.stop(); qDebug()<<"Correct response: " << m_request; } void MyDevice::stateMachine() { m_responseStatus = static_cast<Response::Status>(m_request.mid(2,1).toInt(nullptr, 16)); switch (m_state) { case State::state::STATUS_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::POLL,m_data)); m_state = State::state::STATUS_RESPONSE; break; case State::state::STATUS_RESPONSE: if(m_responseStatus == Response::Status::POWER_UP) m_state = State::state::VERSION_REQUEST; if(m_responseStatus == Response::Status::POWER_UP_WITH_BILL_IN_ACCEPTOR) m_state = State::state::RESET_REQUEST; if(m_responseStatus == Response::Status::POWER_UP_WITH_BILL_IN_STACKER) m_state = State::state::RESET_REQUEST; if(m_responseStatus == Response::Status::INITIALIZE) m_state = State::state::EN_DIS_REQUEST; if(m_responseStatus == Response::Status::ENABLE) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::ACCEPTING) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::ESCROW) m_state = State::state::STACK1_REQUEST; if(m_responseStatus == Response::Status::STACKING) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::STACKED) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::VALID) m_state = State::state::ACK_REQUEST; if(m_responseStatus == Response::Status::REJECTING) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::JAM_IN_ACCEPTOR) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::VERSION_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::GET_VERSION,m_data)); m_state = State::state::VERSION_RESPONSE; break; case State::state::VERSION_RESPONSE: if(m_responseStatus == Response::Status::STAT_VERSION) m_state = State::state::RESET_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::RESET_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::RESET,m_data)); m_state = State::state::RESET_RESPONSE; break; case State::state::RESET_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::EN_DIS_REQUEST: timerResponse.start(m_timeout); clearBuffers(); m_data = QByteArray("\xff\xff",2); serial->write(createMessage(Commands::deviceCommand::ENABLE,m_data)); m_state = State::state::EN_DIS_RESPONSE; break; case State::state::EN_DIS_RESPONSE: if(m_responseStatus == Response::Status::STAT_EN_DIS) m_state = State::state::SECURITY_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::SECURITY_REQUEST: timerResponse.start(m_timeout); clearBuffers(); m_data = QByteArray("\xff\xff",2); serial->write(createMessage(Commands::deviceCommand::SECURITY,m_data)); m_state = State::state::SECURITY_REQUEST; break; case State::state::SECURITY_RESPONSE: if(m_responseStatus == Response::Status::CMD_ENABLE_SECURITY) m_state = State::state::OPTIONAL_FUN_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::OPTIONAL_FUN_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::OPTIONAL,m_data)); m_state = State::state::OPTIONAL_FUN_RESPONSE; break; case State::state::OPTIONAL_FUN_RESPONSE: if(m_responseStatus == Response::Status::CMD_OPTIONAL) m_state = State::state::INHIBIT_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::INHIBIT_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::INHIBIT,m_data)); m_state = State::state::INHIBIT_RESPONSE; break; case State::state::INHIBIT_RESPONSE: if(m_responseStatus == Response::Status::CMD_INHIBIT) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::STACK1_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::STACK1,m_data)); m_state = State::state::STACK1_RESPONSE; break; case State::state::STACK1_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::STACK2_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::STACK2,m_data)); m_state = State::state::STACK2_RESPONSE; break; case State::state::STACK2_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::ACK_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::ACK,m_data)); m_state = State::state::STATUS_REQUEST; break; case State::state::RETURN_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::RETURN,m_data)); m_state = State::state::RETURN_RESPONSE; break; case State::state::RETURN_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; default: break; } } void MyDevice::clearBuffers() { m_request.clear(); m_data.clear(); } bool MyDevice::messageComplete(const QByteArray &data) const { if(data.size()>=3){ quint8 length=(unsigned char)data[1]; if(length==data.length()) return true; } return false; } bool MyDevice::validateResponse(QByteArray data) { if(data.size()<3) return false; QByteArray crc=data.right(2); std::reverse(crc.begin(),crc.end()); bool ok=false; quint16 messageCRC=crc.toHex().toUInt(&ok,16); if(!ok) return false; data.chop(2); return (messageCRC==crc16(data)); } QByteArray MyDevice::createMessage(const Commands::deviceCommand &cmd, const QByteArray &data) { QByteArray message; message.append(0xFC); if(data.count() > 0) { message.append(0x05+data.count()); } else{ message.append(0x05); } message.append((char)cmd); if(data.count() > 0) { for (int i= 0 ; i< data.count() ; i++) message.append(data[i]); } quint16 crc=crc16(message); QByteArray crcData=QByteArray((char *)&crc,2); message.append(crcData); return message; } quint16 MyDevice::crc16(const QByteArray &data) { unsigned int CRC; unsigned char j; CRC = 0; for(int i=0; i < data.size(); i++) { CRC ^= (unsigned char)data[i]; for(j=0; j < 8; j++) { if(CRC & 0x0001) {CRC >>= 1; CRC ^= POLYNOMIAL;} else CRC >>= 1; } } return CRC; }
@Damian7546
I have an observation. Perhaps it is me/only a matter of approach or preference, but why do you choose to introducewaitForReadyRead()
calls (in a loop) inside the slot handler forreadyRead
signal? It may work, but mixes asynchronous and synchronous approach.I would make
serialRecive()
do nothing ifmessageComplete(m_request)
is false. Just buffer the bytes intom_request
and exit the slot (setm_state
if appropriate). Then next timereadyRead
emitted and slot called append the new bytes and see whether now message completed again. When so, process the bytes received in the buffer, either directly inserialRecive()
, or probably better emit amessageCompleted()
signal of your own from there, and do your processing in a slot on that. Seems cleaner to me, but maybe up to you. -
@Damian7546
I have an observation. Perhaps it is me/only a matter of approach or preference, but why do you choose to introducewaitForReadyRead()
calls (in a loop) inside the slot handler forreadyRead
signal? It may work, but mixes asynchronous and synchronous approach.I would make
serialRecive()
do nothing ifmessageComplete(m_request)
is false. Just buffer the bytes intom_request
and exit the slot (setm_state
if appropriate). Then next timereadyRead
emitted and slot called append the new bytes and see whether now message completed again. When so, process the bytes received in the buffer, either directly inserialRecive()
, or probably better emit amessageCompleted()
signal of your own from there, and do your processing in a slot on that. Seems cleaner to me, but maybe up to you.@JonB said in Serial communication:
It may work, but mixes asynchronous and synchronous approach.
But is also may eat kitten...
Don't do that. Buffer the data read in a member variable and wait for the next readyRead() -
@JonB said in Serial communication:
It may work, but mixes asynchronous and synchronous approach.
But is also may eat kitten...
Don't do that. Buffer the data read in a member variable and wait for the next readyRead()@Christian-Ehrlicher @JonB Thank you for yours suggestions.
I have implemented your comments,suggestions, and now code look like:
#include "mydevice.h" MyDevice::MyDevice(QObject *parent) : QObject{parent} { serial = new QSerialPort(); serial->setPortName("COM9"); serial->setBaudRate(QSerialPort::Baud9600); serial->setDataBits(QSerialPort::Data8); serial->setParity(QSerialPort::EvenParity); serial->open(QIODevice::ReadWrite); connect(serial, &QSerialPort::readyRead, this, &MyDevice::serialRecive); connect(&timerResponse, &QTimer::timeout, this, [&](){ m_timeoutResponse = true; timerResponse.stop(); qDebug()<<"Timeout response";} ); connect(&timerState, &QTimer::timeout, this, &MyDevice::stateMachine); if(!serial->isOpen()){ qInfo() << "Serial port status: " << serial->isOpen(); }else{ //run state machine! m_state = State::state::STATUS_REQUEST; timerState.start(100); } } void MyDevice::serialRecive() { m_request_part.append(serial->readAll()); qDebug() << "Partially received: " << m_request_part; while(!messageComplete(m_request_part)) { return; } bool crcValid=validateResponse(m_request_part); if(!crcValid){ qDebug()<<"Crc error"; m_state = State::state::STATUS_REQUEST; return; } timerResponse.stop(); m_request = m_request_part; qDebug()<<"Correct response: " << m_request; } void MyDevice::stateMachine() { if (m_request.count() > 3) m_responseStatus = static_cast<Response::Status>(m_request.mid(2,1).toInt(nullptr, 16)); else m_responseStatus = Response::Status::UNKNOWN; switch (m_state) { case State::state::STATUS_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::POLL,m_data)); m_state = State::state::STATUS_RESPONSE; break; case State::state::STATUS_RESPONSE: if(m_responseStatus == Response::Status::POWER_UP) m_state = State::state::VERSION_REQUEST; if(m_responseStatus == Response::Status::POWER_UP_WITH_BILL_IN_ACCEPTOR) m_state = State::state::RESET_REQUEST; if(m_responseStatus == Response::Status::POWER_UP_WITH_BILL_IN_STACKER) m_state = State::state::RESET_REQUEST; if(m_responseStatus == Response::Status::INITIALIZE) m_state = State::state::EN_DIS_REQUEST; if(m_responseStatus == Response::Status::ENABLE) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::ACCEPTING) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::ESCROW) m_state = State::state::STACK1_REQUEST; if(m_responseStatus == Response::Status::STACKING) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::STACKED) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::VALID) m_state = State::state::ACK_REQUEST; if(m_responseStatus == Response::Status::REJECTING) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::JAM_IN_ACCEPTOR) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::VERSION_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::GET_VERSION,m_data)); m_state = State::state::VERSION_RESPONSE; break; case State::state::VERSION_RESPONSE: if(m_responseStatus == Response::Status::STAT_VERSION) m_state = State::state::RESET_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::RESET_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::RESET,m_data)); m_state = State::state::RESET_RESPONSE; break; case State::state::RESET_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::EN_DIS_REQUEST: timerResponse.start(m_timeout); clearBuffers(); m_data = QByteArray("\xff\xff",2); serial->write(createMessage(Commands::deviceCommand::ENABLE,m_data)); m_state = State::state::EN_DIS_RESPONSE; break; case State::state::EN_DIS_RESPONSE: if(m_responseStatus == Response::Status::STAT_EN_DIS) m_state = State::state::SECURITY_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::SECURITY_REQUEST: timerResponse.start(m_timeout); clearBuffers(); m_data = QByteArray("\xff\xff",2); serial->write(createMessage(Commands::deviceCommand::SECURITY,m_data)); m_state = State::state::SECURITY_REQUEST; break; case State::state::SECURITY_RESPONSE: if(m_responseStatus == Response::Status::CMD_ENABLE_SECURITY) m_state = State::state::OPTIONAL_FUN_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::OPTIONAL_FUN_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::OPTIONAL,m_data)); m_state = State::state::OPTIONAL_FUN_RESPONSE; break; case State::state::OPTIONAL_FUN_RESPONSE: if(m_responseStatus == Response::Status::CMD_OPTIONAL) m_state = State::state::INHIBIT_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::INHIBIT_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::INHIBIT,m_data)); m_state = State::state::INHIBIT_RESPONSE; break; case State::state::INHIBIT_RESPONSE: if(m_responseStatus == Response::Status::CMD_INHIBIT) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::STACK1_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::STACK1,m_data)); m_state = State::state::STACK1_RESPONSE; break; case State::state::STACK1_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::STACK2_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::STACK2,m_data)); m_state = State::state::STACK2_RESPONSE; break; case State::state::STACK2_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::ACK_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::ACK,m_data)); m_state = State::state::STATUS_REQUEST; break; case State::state::RETURN_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::RETURN,m_data)); m_state = State::state::RETURN_RESPONSE; break; case State::state::RETURN_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; default: break; } } void MyDevice::clearBuffers() { m_request.clear(); m_data.clear(); m_request_part.clear(); } bool MyDevice::messageComplete(const QByteArray &data) const { if(data.size()>=3){ quint8 length=(unsigned char)data[1]; if(length==data.length()) return true; } return false; } bool MyDevice::validateResponse(QByteArray data) { if(data.size()<3) return false; QByteArray crc=data.right(2); std::reverse(crc.begin(),crc.end()); bool ok=false; quint16 messageCRC=crc.toHex().toUInt(&ok,16); if(!ok) return false; data.chop(2); return (messageCRC==crc16(data)); } QByteArray MyDevice::createMessage(const Commands::deviceCommand &cmd, const QByteArray &data) { QByteArray message; message.append(0xFC); if(data.count() > 0) { message.append(0x05+data.count()); } else{ message.append(0x05); } message.append((char)cmd); if(data.count() > 0) { for (int i= 0 ; i< data.count() ; i++) message.append(data[i]); } quint16 crc=crc16(message); QByteArray crcData=QByteArray((char *)&crc,2); message.append(crcData); return message; } quint16 MyDevice::crc16(const QByteArray &data) { unsigned int CRC; unsigned char j; CRC = 0; for(int i=0; i < data.size(); i++) { CRC ^= (unsigned char)data[i]; for(j=0; j < 8; j++) { if(CRC & 0x0001) {CRC >>= 1; CRC ^= POLYNOMIAL;} else CRC >>= 1; } } return CRC; }
I would be rally gratefull for more suggestions.
-
@Christian-Ehrlicher @JonB Thank you for yours suggestions.
I have implemented your comments,suggestions, and now code look like:
#include "mydevice.h" MyDevice::MyDevice(QObject *parent) : QObject{parent} { serial = new QSerialPort(); serial->setPortName("COM9"); serial->setBaudRate(QSerialPort::Baud9600); serial->setDataBits(QSerialPort::Data8); serial->setParity(QSerialPort::EvenParity); serial->open(QIODevice::ReadWrite); connect(serial, &QSerialPort::readyRead, this, &MyDevice::serialRecive); connect(&timerResponse, &QTimer::timeout, this, [&](){ m_timeoutResponse = true; timerResponse.stop(); qDebug()<<"Timeout response";} ); connect(&timerState, &QTimer::timeout, this, &MyDevice::stateMachine); if(!serial->isOpen()){ qInfo() << "Serial port status: " << serial->isOpen(); }else{ //run state machine! m_state = State::state::STATUS_REQUEST; timerState.start(100); } } void MyDevice::serialRecive() { m_request_part.append(serial->readAll()); qDebug() << "Partially received: " << m_request_part; while(!messageComplete(m_request_part)) { return; } bool crcValid=validateResponse(m_request_part); if(!crcValid){ qDebug()<<"Crc error"; m_state = State::state::STATUS_REQUEST; return; } timerResponse.stop(); m_request = m_request_part; qDebug()<<"Correct response: " << m_request; } void MyDevice::stateMachine() { if (m_request.count() > 3) m_responseStatus = static_cast<Response::Status>(m_request.mid(2,1).toInt(nullptr, 16)); else m_responseStatus = Response::Status::UNKNOWN; switch (m_state) { case State::state::STATUS_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::POLL,m_data)); m_state = State::state::STATUS_RESPONSE; break; case State::state::STATUS_RESPONSE: if(m_responseStatus == Response::Status::POWER_UP) m_state = State::state::VERSION_REQUEST; if(m_responseStatus == Response::Status::POWER_UP_WITH_BILL_IN_ACCEPTOR) m_state = State::state::RESET_REQUEST; if(m_responseStatus == Response::Status::POWER_UP_WITH_BILL_IN_STACKER) m_state = State::state::RESET_REQUEST; if(m_responseStatus == Response::Status::INITIALIZE) m_state = State::state::EN_DIS_REQUEST; if(m_responseStatus == Response::Status::ENABLE) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::ACCEPTING) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::ESCROW) m_state = State::state::STACK1_REQUEST; if(m_responseStatus == Response::Status::STACKING) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::STACKED) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::VALID) m_state = State::state::ACK_REQUEST; if(m_responseStatus == Response::Status::REJECTING) m_state = State::state::STATUS_REQUEST; if(m_responseStatus == Response::Status::JAM_IN_ACCEPTOR) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::VERSION_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::GET_VERSION,m_data)); m_state = State::state::VERSION_RESPONSE; break; case State::state::VERSION_RESPONSE: if(m_responseStatus == Response::Status::STAT_VERSION) m_state = State::state::RESET_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::RESET_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::RESET,m_data)); m_state = State::state::RESET_RESPONSE; break; case State::state::RESET_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::EN_DIS_REQUEST: timerResponse.start(m_timeout); clearBuffers(); m_data = QByteArray("\xff\xff",2); serial->write(createMessage(Commands::deviceCommand::ENABLE,m_data)); m_state = State::state::EN_DIS_RESPONSE; break; case State::state::EN_DIS_RESPONSE: if(m_responseStatus == Response::Status::STAT_EN_DIS) m_state = State::state::SECURITY_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::SECURITY_REQUEST: timerResponse.start(m_timeout); clearBuffers(); m_data = QByteArray("\xff\xff",2); serial->write(createMessage(Commands::deviceCommand::SECURITY,m_data)); m_state = State::state::SECURITY_REQUEST; break; case State::state::SECURITY_RESPONSE: if(m_responseStatus == Response::Status::CMD_ENABLE_SECURITY) m_state = State::state::OPTIONAL_FUN_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::OPTIONAL_FUN_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::OPTIONAL,m_data)); m_state = State::state::OPTIONAL_FUN_RESPONSE; break; case State::state::OPTIONAL_FUN_RESPONSE: if(m_responseStatus == Response::Status::CMD_OPTIONAL) m_state = State::state::INHIBIT_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::INHIBIT_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::INHIBIT,m_data)); m_state = State::state::INHIBIT_RESPONSE; break; case State::state::INHIBIT_RESPONSE: if(m_responseStatus == Response::Status::CMD_INHIBIT) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::STACK1_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::STACK1,m_data)); m_state = State::state::STACK1_RESPONSE; break; case State::state::STACK1_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::STACK2_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::STACK2,m_data)); m_state = State::state::STACK2_RESPONSE; break; case State::state::STACK2_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; case State::state::ACK_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::ACK,m_data)); m_state = State::state::STATUS_REQUEST; break; case State::state::RETURN_REQUEST: timerResponse.start(m_timeout); clearBuffers(); serial->write(createMessage(Commands::deviceCommand::RETURN,m_data)); m_state = State::state::RETURN_RESPONSE; break; case State::state::RETURN_RESPONSE: if(m_responseStatus == Response::Status::ACK) m_state = State::state::STATUS_REQUEST; if(m_timeoutResponse) { m_timeoutResponse = false; m_state = State::state::STATUS_REQUEST; } break; default: break; } } void MyDevice::clearBuffers() { m_request.clear(); m_data.clear(); m_request_part.clear(); } bool MyDevice::messageComplete(const QByteArray &data) const { if(data.size()>=3){ quint8 length=(unsigned char)data[1]; if(length==data.length()) return true; } return false; } bool MyDevice::validateResponse(QByteArray data) { if(data.size()<3) return false; QByteArray crc=data.right(2); std::reverse(crc.begin(),crc.end()); bool ok=false; quint16 messageCRC=crc.toHex().toUInt(&ok,16); if(!ok) return false; data.chop(2); return (messageCRC==crc16(data)); } QByteArray MyDevice::createMessage(const Commands::deviceCommand &cmd, const QByteArray &data) { QByteArray message; message.append(0xFC); if(data.count() > 0) { message.append(0x05+data.count()); } else{ message.append(0x05); } message.append((char)cmd); if(data.count() > 0) { for (int i= 0 ; i< data.count() ; i++) message.append(data[i]); } quint16 crc=crc16(message); QByteArray crcData=QByteArray((char *)&crc,2); message.append(crcData); return message; } quint16 MyDevice::crc16(const QByteArray &data) { unsigned int CRC; unsigned char j; CRC = 0; for(int i=0; i < data.size(); i++) { CRC ^= (unsigned char)data[i]; for(j=0; j < 8; j++) { if(CRC & 0x0001) {CRC >>= 1; CRC ^= POLYNOMIAL;} else CRC >>= 1; } } return CRC; }
I would be rally gratefull for more suggestions.
@Damian7546 said in Serial communication:
while(!messageComplete(m_request_part)) {
return;
}Why do you have a loop here if you anyway do a return?
-
@Damian7546 said in Serial communication:
while(!messageComplete(m_request_part)) {
return;
}Why do you have a loop here if you anyway do a return?
@jsulm Better?
if(!messageComplete(m_request_part)) { return; }
-
@jsulm Better?
if(!messageComplete(m_request_part)) { return; }
@Damian7546 More readable/clear :)
-
@Damian7546 More readable/clear :)
From my pov the parsing is incorrect as there may be more than one message in the buffer so the second (or even the first part of the second) is thrown away.
-
From my pov the parsing is incorrect as there may be more than one message in the buffer so the second (or even the first part of the second) is thrown away.
@Christian-Ehrlicher so how to fix it?
-
@Christian-Ehrlicher so how to fix it?
@Damian7546
Remove only what is used for a message from the buffer, leaving any further unused bytes there might be for appending to create next "message". -
@Damian7546 Something like this. But better don't use c-style casts:
m_responseStatus = static_cast<Response::Status>(m_request.mid(2,1).toInt(nullptr, 16));
@jsulm
This conversion doesn't work.class Response { public: enum class Status : quint8{ DISABLE = 0x1A, }; Response(); };
In
m_responseStatus
variable I have0x1A
value, but below statement is not equal:m_responseStatus = static_cast<Response::Status>(m_request.mid(2,1).toInt(nullptr, 16)); qDebug() << "m_responseStatus: " << m_request.mid(2,1); if(Response::Status::DISABLE == m_responseStatus) qDebug() << "ok convert"; else qDebug() << "bad convert";
Problem is in
toInt
conversion:qDebug() << "m_responseStatus: " << m_request.mid(2,1); qDebug() << "m_responseStatus: " << m_request.mid(2,1).toInt(nullptr, 16);
result:
m_responseStatus: "\x1A"
m_responseStatus: 0
-
@jsulm
This conversion doesn't work.class Response { public: enum class Status : quint8{ DISABLE = 0x1A, }; Response(); };
In
m_responseStatus
variable I have0x1A
value, but below statement is not equal:m_responseStatus = static_cast<Response::Status>(m_request.mid(2,1).toInt(nullptr, 16)); qDebug() << "m_responseStatus: " << m_request.mid(2,1); if(Response::Status::DISABLE == m_responseStatus) qDebug() << "ok convert"; else qDebug() << "bad convert";
Problem is in
toInt
conversion:qDebug() << "m_responseStatus: " << m_request.mid(2,1); qDebug() << "m_responseStatus: " << m_request.mid(2,1).toInt(nullptr, 16);
result:
m_responseStatus: "\x1A"
m_responseStatus: 0
@Damian7546 said in Serial communication:
In m_responseStatus variable I have 0x1A value, but below statement is not equal:
Sorry, I don't believe that. Start with:
qDebug() << int(m_responseStatus) << int(Response::Status::DISABLE) << 0x1A;
[I assume
int()
is good enough here to test. You can gostatic_cast<int>(...)
if necessary.] -
@Damian7546 said in Serial communication:
In m_responseStatus variable I have 0x1A value, but below statement is not equal:
Sorry, I don't believe that. Start with:
qDebug() << int(m_responseStatus) << int(Response::Status::DISABLE) << 0x1A;
[I assume
int()
is good enough here to test. You can gostatic_cast<int>(...)
if necessary.]@JonB said in Serial communication:
Sorry, I don't believe that. Start with:
qDebug() << int(m_responseStatus) << int(Response::Status::DISABLE) << 0x1A;Result:
0 26 26
-
@JonB said in Serial communication:
Sorry, I don't believe that. Start with:
qDebug() << int(m_responseStatus) << int(Response::Status::DISABLE) << 0x1A;Result:
0 26 26
@Damian7546
So you have your answer from the first 2 outputs.0 != 26
, the two values being compared are indeed not equal/the same.Behaviour is quite correct given your
m_request.mid(2,1).toInt(nullptr, 16);
, which (QByteArray::toInt()) is quite incorrect to use on your byte array containing a byte with value\x16
(26).toInt()
is not for use in your case, as you do not have a string representation of an integer to convert. -
@Damian7546
So you have your answer from the first 2 outputs.0 != 26
, the two values being compared are indeed not equal/the same.Behaviour is quite correct given your
m_request.mid(2,1).toInt(nullptr, 16);
, which (QByteArray::toInt()) is quite incorrect to use on your byte array containing a byte with value\x16
(26).toInt()
is not for use in your case, as you do not have a string representation of an integer to convert.@JonB So ho to compare
Response::Status::DISABLE
to byte in QByteArraym_request[2]
? -
@JonB So ho to compare
Response::Status::DISABLE
to byte in QByteArraym_request[2]
?@Damian7546
Exactly as you have just written it!qDebug() << Response::Status::DISABLE << m_request[2];
[You may have to do casting.] Sightly better is to use
m_request.at(2)
in place ofm_request[2]
here, but that is a detail and not related to your issue. You want to inspect the actual bytes in theQByteArray
, not callQByteArray::toInt()
. -
@Damian7546
Exactly as you have just written it!qDebug() << Response::Status::DISABLE << m_request[2];
[You may have to do casting.] Sightly better is to use
m_request.at(2)
in place ofm_request[2]
here, but that is a detail and not related to your issue. You want to inspect the actual bytes in theQByteArray
, not callQByteArray::toInt()
. -
@JonB casting is highly suggested, as
Response::Status
is an uint_8t and at() returns a char, aka int8_t@J-Hilk
Yes, but what I meant is I don't have anything available to test what compiler says as I write answers. So that detail is left to OP :) Since the OP asks about comparing (with==
) one should be able to compare aunit_8t
against achar
/int8_t
, I think, using C++ automatic expression promotion without bothering to cast/convert, I think? But maybe not/warning forenum
value against integer?The important thing is no
QByteArray::toInt()
here! -
@J-Hilk
Yes, but what I meant is I don't have anything available to test what compiler says as I write answers. So that detail is left to OP :) Since the OP asks about comparing (with==
) one should be able to compare aunit_8t
against achar
/int8_t
, I think, using C++ automatic expression promotion without bothering to cast/convert, I think? But maybe not/warning forenum
value against integer?The important thing is no
QByteArray::toInt()
here!@JonB It doesn't work:
qDebug() << static_cast<int>(Response::Status::DISABLE) << m_request.at(2);
Result:
ASSERT: "uint(i) < uint(size())" in file C:/Qt/5.15.2/mingw81_32/include/QtCore/qbytearray.h, line 500
And this way:
qDebug() << static_cast<int>(Response::Status::DISABLE) << m_request[2];
Result:
Where :
m_request response: "\xFC\x05\x1A\xF4\xE8"