From c4487f05f11005167310c6377d1e7f4d40d55986 Mon Sep 17 00:00:00 2001 From: Peter Svihra <peter.svihra@cern.ch> Date: Tue, 21 Apr 2020 16:30:35 +0200 Subject: [PATCH] changed to static allocation --- arduino/common/lib/CommsControl/CommsCommon.h | 4 +- .../common/lib/CommsControl/CommsControl.cpp | 158 ++++++++---------- .../common/lib/CommsControl/CommsControl.h | 24 +-- .../common/lib/CommsControl/CommsFormat.cpp | 51 +++--- arduino/common/lib/CommsControl/CommsFormat.h | 17 +- 5 files changed, 122 insertions(+), 132 deletions(-) diff --git a/arduino/common/lib/CommsControl/CommsCommon.h b/arduino/common/lib/CommsControl/CommsCommon.h index 90d2d01c..1dc8174f 100644 --- a/arduino/common/lib/CommsControl/CommsCommon.h +++ b/arduino/common/lib/CommsControl/CommsCommon.h @@ -8,8 +8,8 @@ #define CONST_TIMEOUT_CMD 50 -#define CONST_MAX_SIZE_RB_RECEIVING 10 -#define CONST_MAX_SIZE_RB_SENDING 5 +#define CONST_MAX_SIZE_RB_RECEIVING 2 +#define CONST_MAX_SIZE_RB_SENDING 2 #define CONST_MAX_SIZE_PACKET 64 #define CONST_MAX_SIZE_BUFFER 128 #define CONST_MIN_SIZE_PACKET 7 diff --git a/arduino/common/lib/CommsControl/CommsControl.cpp b/arduino/common/lib/CommsControl/CommsControl.cpp index e2a8e327..91b3fa41 100644 --- a/arduino/common/lib/CommsControl/CommsControl.cpp +++ b/arduino/common/lib/CommsControl/CommsControl.cpp @@ -15,22 +15,21 @@ CommsControl::CommsControl(uint32_t baudrate) { memset(_comms_received, 0, sizeof(_comms_received)); memset(_comms_send , 0, sizeof(_comms_send )); - _ring_buff_alarm = new RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING>(); - _ring_buff_data = new RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING>(); - _ring_buff_cmd = new RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING>(); + _ring_buff_alarm = RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING>(); + _ring_buff_data = RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING>(); + _ring_buff_cmd = RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING>(); - _ring_buff_received = new RingBuf<Payload, CONST_MAX_SIZE_RB_RECEIVING>(); + _ring_buff_received = RingBuf<Payload, CONST_MAX_SIZE_RB_RECEIVING>(); _comms_tmp = CommsFormat(CONST_MAX_SIZE_PACKET - CONST_MIN_SIZE_PACKET ); - _comms_ack = CommsFormat::generateACK(); - _comms_nck = CommsFormat::generateNACK(); + CommsFormat::generateACK(_comms_ack); + CommsFormat::generateNACK(_comms_nck); _sequence_send = 0; _sequence_receive = 0; } -// WIP CommsControl::~CommsControl() { ; } @@ -40,26 +39,26 @@ void CommsControl::beginSerial() { } // main function to always call and try and send data -// TODO: needs switch on data type with global timeouts on data pushing +// _last_trans_time is changed when transmission occurs in sendQueue void CommsControl::sender() { uint32_t tnow = static_cast<uint32_t>(millis()); if (tnow - _last_trans_time > CONST_TIMEOUT_ALARM) { - sendQueue(_ring_buff_alarm); + sendQueue(&_ring_buff_alarm); } if (tnow - _last_trans_time > CONST_TIMEOUT_CMD) { - sendQueue(_ring_buff_cmd); + sendQueue(&_ring_buff_cmd); } if (tnow - _last_trans_time > CONST_TIMEOUT_DATA) { - sendQueue(_ring_buff_data); + sendQueue(&_ring_buff_data); } } // main function to always try to receive data // TODO: needs switch on data type with global timeouts on data pushing void CommsControl::receiver() { - uint8_t currentTransIndex; + uint8_t current_trans_index; // check if any data in waiting if (Serial.available()) { @@ -72,14 +71,14 @@ void CommsControl::receiver() { // if managed to read at least 1 byte if (_last_trans_index > 0 && _last_trans_index < CONST_MAX_SIZE_BUFFER) { - currentTransIndex = _last_trans_index - 1; + current_trans_index = _last_trans_index - 1; // find the boundary of frames - if (_last_trans[currentTransIndex] == COMMS_FRAME_BOUNDARY) { + if (_last_trans[current_trans_index] == COMMS_FRAME_BOUNDARY) { // if not found start or if read the same byte as last time - if (!_found_start || _start_trans_index == currentTransIndex) { + if (!_found_start || _start_trans_index == current_trans_index) { _found_start = true; - _start_trans_index = currentTransIndex; + _start_trans_index = current_trans_index; } else { // if managed to decode and compare CRC if (decoder(_last_trans, _start_trans_index, _last_trans_index)) { @@ -103,16 +102,16 @@ void CommsControl::receiver() { finishPacket(type); break; default: - uint8_t tmpSequenceReceive = (control >> 1 ) & 0x7F; - tmpSequenceReceive += 1; + uint8_t sequence_receive = (control >> 1 ) & 0x7F; + sequence_receive += 1; // received DATA if (receivePacket(type)) { - _comms_ack->setAddress(_comms_tmp.getAddress()); - _comms_ack->setSequenceReceive(tmpSequenceReceive); + _comms_ack.setAddress(_comms_tmp.getAddress()); + _comms_ack.setSequenceReceive(sequence_receive); sendPacket(_comms_ack); } else { - _comms_nck->setAddress(_comms_tmp.getAddress()); - _comms_nck->setSequenceReceive(tmpSequenceReceive); + _comms_nck.setAddress(_comms_tmp.getAddress()); + _comms_nck.setSequenceReceive(sequence_receive); sendPacket(_comms_nck); } @@ -137,43 +136,30 @@ void CommsControl::receiver() { } bool CommsControl::writePayload(Payload &pl) { - CommsFormat* tmpComms; - PAYLOAD_TYPE type = pl.getType(); - - // switch on different received payload types - // TODO simplify the static functions - switch(type) { - case ALARM: - tmpComms = CommsFormat::generateALARM(&pl); - break; - case DATA: - tmpComms = CommsFormat::generateDATA(&pl); - break; - case CMD: - tmpComms = CommsFormat::generateCMD(&pl); - break; - default: - return false; - } - - RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING> *queue = getQueue(type); - // add new entry to the queue - if (queue->isFull()) { - CommsFormat *tmpCommsRm; - if (queue->pop(tmpCommsRm)) { - delete tmpCommsRm; + PAYLOAD_TYPE payload_type = pl.getType(); + if (payload_type != PAYLOAD_TYPE::UNSET) { + // create comms format using payload, the type is actually deduced from the payload + CommsFormat comms = CommsFormat(pl); + + RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING> *queue = getQueue(payload_type); + // add new entry to the queue + if (queue->isFull()) { + CommsFormat comms_rm; + if (queue->pop(comms_rm)) { + ; + } } - } - if (queue->push(tmpComms) ) { - return true; + if (queue->push(comms) ) { + return true; + } } return false; } bool CommsControl::readPayload( Payload &pl) { - if ( !_ring_buff_received->isEmpty()) { - if (_ring_buff_received->pop(pl)) { + if ( !_ring_buff_received.isEmpty()) { + if (_ring_buff_received.pop(pl)) { return true; } } @@ -181,13 +167,13 @@ bool CommsControl::readPayload( Payload &pl) { } // general encoder of any transmission -bool CommsControl::encoder(uint8_t *data, uint8_t dataSize) { - if (dataSize > 0) { +bool CommsControl::encoder(uint8_t *data, uint8_t data_size) { + if (data_size > 0) { _comms_send_size = 0; uint8_t tmpVal = 0; _comms_send[_comms_send_size++] = data[0]; - for (uint8_t idx = 1; idx < dataSize - 1; idx++) { + for (uint8_t idx = 1; idx < data_size - 1; idx++) { tmpVal = data[idx]; if (tmpVal == COMMS_FRAME_ESCAPE || tmpVal == COMMS_FRAME_BOUNDARY) { _comms_send[_comms_send_size++] = COMMS_FRAME_ESCAPE; @@ -195,7 +181,7 @@ bool CommsControl::encoder(uint8_t *data, uint8_t dataSize) { } _comms_send[_comms_send_size++] = tmpVal; } - _comms_send[_comms_send_size++] = data[dataSize-1]; + _comms_send[_comms_send_size++] = data[data_size-1]; return true; } @@ -204,23 +190,23 @@ bool CommsControl::encoder(uint8_t *data, uint8_t dataSize) { // general decoder of any transmission -bool CommsControl::decoder(uint8_t* data, uint8_t dataStart, uint8_t dataStop) { +bool CommsControl::decoder(uint8_t* data, uint8_t data_start, uint8_t data_stop) { // need to have more than 1 byte transferred - if (dataStop > (dataStart + 1)) { + if (data_stop > (data_start + 1)) { _comms_received_size = 0; - uint8_t tmpVal = 0; + uint8_t tmp_val = 0; bool escaped = false; - for (uint8_t idx = dataStart; idx < dataStop; idx++) { - tmpVal = data[idx]; - if (tmpVal == COMMS_FRAME_ESCAPE) { + for (uint8_t idx = data_start; idx < data_stop; idx++) { + tmp_val = data[idx]; + if (tmp_val == COMMS_FRAME_ESCAPE) { escaped = true; } else { if (escaped) { - tmpVal ^= (1 << COMMS_ESCAPE_BIT_SWAP); + tmp_val ^= (1 << COMMS_ESCAPE_BIT_SWAP); escaped = false; } - _comms_received[_comms_received_size++] = tmpVal; + _comms_received[_comms_received_size++] = tmp_val; } } _comms_tmp.copyData(_comms_received, _comms_received_size); @@ -230,10 +216,10 @@ bool CommsControl::decoder(uint8_t* data, uint8_t dataStart, uint8_t dataStop) { } // sending anything of commsDATA format -void CommsControl::sendQueue(RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING> *queue) { +void CommsControl::sendQueue(RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING> *queue) { // if have data to send if (!queue->isEmpty()) { - queue->operator [](0)->setSequenceSend(_sequence_send); + queue->operator [](0).setSequenceSend(_sequence_send); sendPacket(queue->operator [](0)); // reset sending counter @@ -241,9 +227,9 @@ void CommsControl::sendQueue(RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING> * } } -void CommsControl::sendPacket(CommsFormat *packet) { +void CommsControl::sendPacket(CommsFormat &packet) { // if encoded and able to write data - if (encoder(packet->getData(), packet->getSize()) ) { + if (encoder(packet.getData(), packet.getSize()) ) { if (Serial.availableForWrite() >= _comms_send_size) { Serial.write(_comms_send, _comms_send_size); } @@ -252,7 +238,7 @@ void CommsControl::sendPacket(CommsFormat *packet) { // resending the packet, can lower the timeout since either NACK or wrong FCS already checked //WIP -void CommsControl::resendPacket(RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING> *queue) { +void CommsControl::resendPacket(RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING> *queue) { ; } @@ -261,35 +247,35 @@ void CommsControl::resendPacket(RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING bool CommsControl::receivePacket(PAYLOAD_TYPE &type) { _payload_tmp.unsetAll(); _payload_tmp.setType(type); - void *tmpInformation = _payload_tmp.getInformation(); + void *information = _payload_tmp.getInformation(); // if type is definet, copy information from comms to payload - if (tmpInformation != nullptr) { - memcpy(tmpInformation, _comms_tmp.getInformation(), _comms_tmp.getInfoSize()); + if (information != nullptr) { + memcpy(information, _comms_tmp.getInformation(), _comms_tmp.getInfoSize()); // remove first entry if queue is full - if (_ring_buff_received->isFull()) { - Payload tmpPlRm; - if (_ring_buff_received->pop(tmpPlRm)) { + if (_ring_buff_received.isFull()) { + Payload payload_rm; + if (_ring_buff_received.pop(payload_rm)) { ; } } - return _ring_buff_received->push(_payload_tmp); + return _ring_buff_received.push(_payload_tmp); } return false; } // if FCS is ok, remove from queue void CommsControl::finishPacket(PAYLOAD_TYPE &type) { - RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING> *tmpQueue = getQueue(type); + RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING> *queue = getQueue(type); - if (tmpQueue != nullptr && !tmpQueue->isEmpty()) { + if (queue != nullptr && !queue->isEmpty()) { // get the sequence send from first entry in the queue, add one as that should be return // 0x7F to deal with possible overflows (0 should follow after 127) - if (((tmpQueue->operator [](0)->getSequenceSend() + 1) & 0x7F) == _sequence_receive) { + if (((queue->operator [](0).getSequenceSend() + 1) & 0x7F) == _sequence_receive) { _sequence_send = (_sequence_send + 1) % 128; - CommsFormat * tmpComms; - if (tmpQueue->pop(tmpComms)) { - delete tmpComms; + CommsFormat comms_rm; + if (queue->pop(comms_rm)) { + ; } } } @@ -309,14 +295,14 @@ PAYLOAD_TYPE CommsControl::getInfoType(uint8_t *address) { } // get link to queue according to packet format -RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING> *CommsControl::getQueue(PAYLOAD_TYPE &type) { +RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING> *CommsControl::getQueue(PAYLOAD_TYPE &type) { switch (type) { case PAYLOAD_TYPE::ALARM: - return _ring_buff_alarm; + return &_ring_buff_alarm; case PAYLOAD_TYPE::CMD: - return _ring_buff_cmd; + return &_ring_buff_cmd; case PAYLOAD_TYPE::DATA: - return _ring_buff_data; + return &_ring_buff_data; default: return nullptr; } diff --git a/arduino/common/lib/CommsControl/CommsControl.h b/arduino/common/lib/CommsControl/CommsControl.h index a01cf726..13c30a7e 100644 --- a/arduino/common/lib/CommsControl/CommsControl.h +++ b/arduino/common/lib/CommsControl/CommsControl.h @@ -26,31 +26,31 @@ public: void receiver(); private: - RingBuf<CommsFormat *,CONST_MAX_SIZE_RB_SENDING> *getQueue(PAYLOAD_TYPE &type); + RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING> *getQueue(PAYLOAD_TYPE &type); PAYLOAD_TYPE getInfoType(uint8_t *address); - void sendQueue (RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING> *queue); - void resendPacket (RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING> *queue); + void sendQueue (RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING> *queue); + void resendPacket (RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING> *queue); bool receivePacket(PAYLOAD_TYPE &type); void finishPacket (PAYLOAD_TYPE &type); - bool encoder(uint8_t* payload, uint8_t dataSize); - bool decoder(uint8_t* payload, uint8_t dataStart, uint8_t dataStop); + bool encoder(uint8_t* payload, uint8_t data_size); + bool decoder(uint8_t* payload, uint8_t dataStart, uint8_t data_stop); - void sendPacket(CommsFormat* packet); + void sendPacket(CommsFormat &packet); private: uint8_t _sequence_send; uint8_t _sequence_receive; - CommsFormat* _comms_ack; - CommsFormat* _comms_nck; + CommsFormat _comms_ack; + CommsFormat _comms_nck; - RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING> *_ring_buff_alarm; - RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING> *_ring_buff_data; - RingBuf<CommsFormat *, CONST_MAX_SIZE_RB_SENDING> *_ring_buff_cmd; + RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING> _ring_buff_alarm; + RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING> _ring_buff_data; + RingBuf<CommsFormat, CONST_MAX_SIZE_RB_SENDING> _ring_buff_cmd; - RingBuf<Payload, CONST_MAX_SIZE_RB_RECEIVING> *_ring_buff_received; + RingBuf<Payload, CONST_MAX_SIZE_RB_RECEIVING> _ring_buff_received; Payload _payload_tmp; CommsFormat _comms_tmp; diff --git a/arduino/common/lib/CommsControl/CommsFormat.cpp b/arduino/common/lib/CommsControl/CommsFormat.cpp index 3778945a..6c2e16e0 100644 --- a/arduino/common/lib/CommsControl/CommsFormat.cpp +++ b/arduino/common/lib/CommsControl/CommsFormat.cpp @@ -1,11 +1,35 @@ #include "CommsFormat.h" // constructor to init variables -CommsFormat::CommsFormat(uint8_t infoSize, uint8_t address, uint16_t control) { +CommsFormat::CommsFormat(uint8_t info_size, uint8_t address, uint16_t control) { + init(info_size, address, control); +} + +CommsFormat::CommsFormat(Payload &pl) { + uint8_t address; + switch (pl.getType()) { + case PAYLOAD_TYPE::ALARM: + address = PACKET_ALARM; + break; + case PAYLOAD_TYPE::CMD: + address = PACKET_CMD; + break; + case PAYLOAD_TYPE::DATA: + address = PACKET_DATA; + break; + default: + address = 0; + break; + } + init(pl.getSize(), address); + setInformation(&pl); +} + +void CommsFormat::init(uint8_t info_size, uint8_t address, uint16_t control) { memset(_data, 0, sizeof(_data)); - _info_size = infoSize; - _packet_size = infoSize + CONST_MIN_SIZE_PACKET ; // minimum size (start,address,control,fcs,stop) + _info_size = info_size; + _packet_size = info_size + CONST_MIN_SIZE_PACKET ; // minimum size (start,address,control,fcs,stop) if (_packet_size > CONST_MAX_SIZE_PACKET) { return; } @@ -20,6 +44,7 @@ CommsFormat::CommsFormat(uint8_t infoSize, uint8_t address, uint16_t control) { generateCrc(); } + void CommsFormat::assignBytes(uint8_t* target, uint8_t* source, uint8_t size, bool calcCrc) { memcpy(target, source, size); if (calcCrc) { @@ -89,23 +114,3 @@ void CommsFormat::copyData(uint8_t* data, uint8_t dataSize) { assignBytes(getData(), data, dataSize); } - - -// STATIC METHODS -// TODO rewrite in a slightly better way using the enum -CommsFormat* CommsFormat::generateALARM(Payload *pl) { - CommsFormat *tmpComms = new CommsFormat(pl->getSize(), PACKET_ALARM); - tmpComms->setInformation(pl); - return tmpComms; -} -CommsFormat* CommsFormat::generateCMD (Payload *pl) { - CommsFormat *tmpComms = new CommsFormat(pl->getSize(), PACKET_CMD ); - tmpComms->setInformation(pl); - return tmpComms; -} -CommsFormat* CommsFormat::generateDATA (Payload *pl) { - CommsFormat *tmpComms = new CommsFormat(pl->getSize(), PACKET_DATA ); - tmpComms->setInformation(pl); - return tmpComms; -} - diff --git a/arduino/common/lib/CommsControl/CommsFormat.h b/arduino/common/lib/CommsControl/CommsFormat.h index ff8916e4..d39716ec 100644 --- a/arduino/common/lib/CommsControl/CommsFormat.h +++ b/arduino/common/lib/CommsControl/CommsFormat.h @@ -13,15 +13,16 @@ // class to provide all needed control in data format class CommsFormat { public: - CommsFormat(uint8_t infoSize = 0, uint8_t address = 0x00, uint16_t control = 0x0000); + CommsFormat(uint8_t info_size = 0, uint8_t address = 0x00, uint16_t control = 0x0000); + CommsFormat(Payload &pl); CommsFormat(const CommsFormat& other) { - _crc = other._crc; + _crc = other._crc; _packet_size = other._packet_size; _info_size = other._info_size; memcpy(_data, other._data, CONST_MAX_SIZE_PACKET); } CommsFormat& operator=(const CommsFormat& other) { - _crc = other._crc; + _crc = other._crc; _packet_size = other._packet_size; _info_size = other._info_size; memcpy(_data, other._data, CONST_MAX_SIZE_PACKET); @@ -59,14 +60,12 @@ public: void copyData(uint8_t* payload, uint8_t dataSize); - static CommsFormat* generateACK() { return new CommsFormat(0, 0, COMMS_CONTROL_ACK << 8); } - static CommsFormat* generateNACK() { return new CommsFormat(0, 0, COMMS_CONTROL_NACK << 8); } - - static CommsFormat* generateALARM(Payload *pl); - static CommsFormat* generateCMD (Payload *pl); - static CommsFormat* generateDATA (Payload *pl); + static void generateACK (CommsFormat &comms) { comms = CommsFormat(0, 0, COMMS_CONTROL_ACK << 8); } + static void generateNACK(CommsFormat &comms) { comms = CommsFormat(0, 0, COMMS_CONTROL_NACK << 8); } private: + void init(uint8_t info_size = 0, uint8_t address = 0x00, uint16_t control = 0x0000); + uint8_t _data[CONST_MAX_SIZE_PACKET]; uint8_t _packet_size; uint8_t _info_size; -- GitLab