From e8a16ac59cf87d27d1e79a6571a6ee11f0edc3c2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 29 Oct 2019 18:21:01 +0100 Subject: [PATCH 1/7] dummy com if sendMessage data const --- devicehandlers/DeviceCommunicationIF.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devicehandlers/DeviceCommunicationIF.h b/devicehandlers/DeviceCommunicationIF.h index e0aca573..88f0d124 100644 --- a/devicehandlers/DeviceCommunicationIF.h +++ b/devicehandlers/DeviceCommunicationIF.h @@ -40,7 +40,7 @@ public: virtual void close(Cookie *cookie) = 0; //SHOULDDO can data be const? - virtual ReturnValue_t sendMessage(Cookie *cookie, uint8_t *data, + virtual ReturnValue_t sendMessage(Cookie *cookie,const uint8_t *data, uint32_t len) = 0; virtual ReturnValue_t getSendSuccess(Cookie *cookie) = 0; From 3887cb8ca14f4a12a2fd446e1779b66c76096962 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 29 Oct 2019 18:22:34 +0100 Subject: [PATCH 2/7] removed wrong include in dhb --- devicehandlers/DeviceHandlerBase.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 2f16f846..1bb411b7 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -11,7 +11,6 @@ #include #include #include -#include object_id_t DeviceHandlerBase::powerSwitcherId = 0; object_id_t DeviceHandlerBase::rawDataReceiverId = 0; From 16af33a7bb52e3d9363e1e6f61e16a67783f74df Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Tue, 29 Oct 2019 19:31:18 +0100 Subject: [PATCH 3/7] doc for fifo, device com if.. --- container/FIFO.h | 4 +-- container/SimpleRingBuffer.h | 28 +++++++++++++++++++++ devicehandlers/DeviceCommunicationIF.h | 35 +++++++++++++++++++++++++- serialize/SerializeAdapter.h | 2 ++ 4 files changed, 66 insertions(+), 3 deletions(-) diff --git a/container/FIFO.h b/container/FIFO.h index 670a50d7..134da9b8 100644 --- a/container/FIFO.h +++ b/container/FIFO.h @@ -21,7 +21,7 @@ public: readIndex(0), writeIndex(0), currentSize(0) { } - bool emtpy() { + bool empty() { return (currentSize == 0); } @@ -45,7 +45,7 @@ public: } ReturnValue_t retrieve(T *value) { - if (emtpy()) { + if (empty()) { return EMPTY; } else { *value = data[readIndex]; diff --git a/container/SimpleRingBuffer.h b/container/SimpleRingBuffer.h index 6d7d67e1..45cb0927 100644 --- a/container/SimpleRingBuffer.h +++ b/container/SimpleRingBuffer.h @@ -4,12 +4,40 @@ #include #include +/** + * Circular buffer implementation, useful for buffering into data streams. + * Note that the deleteData() has to be called to increment the read pointer + */ class SimpleRingBuffer: public RingBufferBase<> { public: SimpleRingBuffer(uint32_t size, bool overwriteOld); virtual ~SimpleRingBuffer(); + + /** + * Write to circular buffer and increment write pointer by amount + * @param data + * @param amount + * @return + */ ReturnValue_t writeData(const uint8_t* data, uint32_t amount); + + /** + * Read from circular buffer at read pointer + * @param data + * @param amount + * @param readRemaining + * @param trueAmount + * @return + */ ReturnValue_t readData(uint8_t* data, uint32_t amount, bool readRemaining = false, uint32_t* trueAmount = NULL); + + /** + * Delete data starting by incrementing read pointer + * @param amount + * @param deleteRemaining + * @param trueAmount + * @return + */ ReturnValue_t deleteData(uint32_t amount, bool deleteRemaining = false, uint32_t* trueAmount = NULL); private: // static const uint8_t TEMP_READ_PTR = 1; diff --git a/devicehandlers/DeviceCommunicationIF.h b/devicehandlers/DeviceCommunicationIF.h index 88f0d124..59420861 100644 --- a/devicehandlers/DeviceCommunicationIF.h +++ b/devicehandlers/DeviceCommunicationIF.h @@ -4,6 +4,22 @@ #include #include +/** + * Documentation: Dissertation Baetz p.138 + * + * This is an interface to decouple device communication from + * the device handler to allow reuse of these components. + * It works with the assumption that received data + * is polled by a component. There are four generic steps of device communication: + * + * 1. Send data to a device + * 2. Get acknowledgement for sending + * 3. Request reading data from a device + * 4. Read received data + * + * To identify different connection over a single interface can return so-called cookies to components. + * + */ class DeviceCommunicationIF: public HasReturnvaluesIF { public: static const uint8_t INTERFACE_ID = CLASS_ID::DEVICE_COMMUNICATION_IF; @@ -39,7 +55,15 @@ public: virtual void close(Cookie *cookie) = 0; - //SHOULDDO can data be const? + /** + * Called by DHB in the SEND_WRITE doSendWrite(). + * This function is used to send data to the physical device + * by implementing and calling related drivers or wrapper functions. + * @param cookie + * @param data + * @param len + * @return + */ virtual ReturnValue_t sendMessage(Cookie *cookie,const uint8_t *data, uint32_t len) = 0; @@ -47,6 +71,15 @@ public: virtual ReturnValue_t requestReceiveMessage(Cookie *cookie) = 0; + /** + * Called by DHB in the GET_WIRTE doGetRead(). + * This function is used to receive data from the physical device + * by implementing and calling related drivers or wrapper functions. + * @param cookie + * @param data + * @param len + * @return + */ virtual ReturnValue_t readReceivedMessage(Cookie *cookie, uint8_t **buffer, uint32_t *size) = 0; diff --git a/serialize/SerializeAdapter.h b/serialize/SerializeAdapter.h index 04216c89..a0aab9e9 100644 --- a/serialize/SerializeAdapter.h +++ b/serialize/SerializeAdapter.h @@ -28,6 +28,8 @@ * This can also be applied to uint32_t and uint64_t: * * 1. Use the AutoSerializeAdapter::deSerialize function with bool bigEndian = true: + * The pointer *buffer will be incremented automatically by the typeSize of data, + * so this function can be called on &buffer without adjusting pointer position * * uint16_t data; * int32_t dataLen = sizeof(data); From 46986f69e4a17923717464c120f4d227df561618 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Sat, 2 Nov 2019 23:30:12 +0100 Subject: [PATCH 4/7] serialize tools more documentation --- serialize/SerialBufferAdapter.cpp | 8 ++++---- serialize/SerialBufferAdapter.h | 8 ++++++++ serialize/SerialFixedArrayListAdapter.h | 11 +++++++++++ serialize/SerialLinkedListAdapter.h | 10 +++++++++- serialize/SerializeIF.h | 6 +++--- 5 files changed, 35 insertions(+), 8 deletions(-) diff --git a/serialize/SerialBufferAdapter.cpp b/serialize/SerialBufferAdapter.cpp index 3ed083bf..c5fcfb57 100644 --- a/serialize/SerialBufferAdapter.cpp +++ b/serialize/SerialBufferAdapter.cpp @@ -5,15 +5,15 @@ template SerialBufferAdapter::SerialBufferAdapter(const uint8_t* buffer, - T bufferLength, bool serializeLenght) : - serializeLength(serializeLenght), constBuffer(buffer), buffer(NULL), bufferLength( + T bufferLength, bool serializeLength) : + serializeLength(serializeLength), constBuffer(buffer), buffer(NULL), bufferLength( bufferLength) { } template SerialBufferAdapter::SerialBufferAdapter(uint8_t* buffer, T bufferLength, - bool serializeLenght) : - serializeLength(serializeLenght), constBuffer(NULL), buffer(buffer), bufferLength( + bool serializeLength) : + serializeLength(serializeLength), constBuffer(NULL), buffer(buffer), bufferLength( bufferLength) { } diff --git a/serialize/SerialBufferAdapter.h b/serialize/SerialBufferAdapter.h index 7cd75d55..7d192bb4 100644 --- a/serialize/SerialBufferAdapter.h +++ b/serialize/SerialBufferAdapter.h @@ -5,6 +5,14 @@ #include /** + * This adapter provides an interface for SerializeIF to serialize or deserialize + * buffers with no length header but a known size. + * + * Additionally, the buffer length can be serialized too and will be put in front of the serialized buffer. + * + * Can be used with SerialLinkedListAdapter by declaring a SerializeElement with + * SerialElement> serialBufferElement + * * \ingroup serialize */ template diff --git a/serialize/SerialFixedArrayListAdapter.h b/serialize/SerialFixedArrayListAdapter.h index 16919b62..821e8710 100644 --- a/serialize/SerialFixedArrayListAdapter.h +++ b/serialize/SerialFixedArrayListAdapter.h @@ -5,6 +5,17 @@ #include /** + * This adapter provides an interface for SerializeIF to serialize and deserialize + * buffers with a header containing the buffer length. + * + * Can be used by SerialLinkedListAdapter. + * + * Buffers with a size header inside that class can be declared with + * SerialFixedArrayListAdapter. + * typeOfMaxData specifies the data type of the buffer header containing the buffer size that follows + * and MAX_BUFFER_LENGTH specifies the maximum allowed value for the buffer size. + * The sequence of objects is defined in the constructor by using the setStart and setNext functions. + * * \ingroup serialize */ template diff --git a/serialize/SerialLinkedListAdapter.h b/serialize/SerialLinkedListAdapter.h index 44d9e15b..08d385ee 100644 --- a/serialize/SerialLinkedListAdapter.h +++ b/serialize/SerialLinkedListAdapter.h @@ -17,12 +17,20 @@ * An alternative to the AutoSerializeAdapter functions to implement the conversion * of object data to data streams or vice-versa, using linked lists. * - * All object members with a datatype are declared as SerializeElement inside the class. + * All object members with a datatype are declared as SerializeElement inside the class + * implementing this adapter. * * Buffers with a size header inside that class can be declared with * SerialFixedArrayListAdapter. * typeOfMaxData specifies the data type of the buffer header containing the buffer size that follows * and MAX_BUFFER_LENGTH specifies the maximum allowed value for the buffer size. + * Please note that a direct link from a linked element to a SerialFixedArrayListAdapter element is not possible and a + * helper needs to be used by calling .setNext(&linkHelper) + * and initialising the link helper as linkHelper(&SerialFixedArrayListAdapterElement). + * + * For buffers with no size header, declare + * SerializeElement> and initialise the buffer adapter in the constructor. + * * The sequence of objects is defined in the constructor by using the setStart and setNext functions. * * The serialization and deserialization process is done by instantiating the class and diff --git a/serialize/SerializeIF.h b/serialize/SerializeIF.h index d86349ab..cada1e72 100644 --- a/serialize/SerializeIF.h +++ b/serialize/SerializeIF.h @@ -11,14 +11,14 @@ /** * An interface for alle classes which require translation of objects data into data streams and vice-versa. * - * If the target architecture is little endian (ARM), any data types created might + * If the target architecture is little endian (e.g. ARM), any data types created might * have the wrong endiness if they are to be used for the FSFW. - * there are three ways to retrieve data out of a buffer to be used in the FSFW to use regular aligned (big endian) data. + * There are three ways to retrieve data out of a buffer to be used in the FSFW to use regular aligned (big endian) data. * This can also be applied to uint32_t and uint64_t: * * 1. Use the @c AutoSerializeAdapter::deSerialize function with @c bigEndian = true * 2. Perform a bitshift operation - * 3. @c memcpy can be used when data is little-endian. Otherwise, @c EndianSwapper has to be used. + * 3. @c memcpy can be used when data is in little-endian format. Otherwise, @c EndianSwapper has to be used in conjuction. * * When serializing for downlink, the packets are generally serialized assuming big endian data format * like seen in TmPacketStored.cpp for example. From 8eb1a5b13e2fccf21176aed9a6a27287196fd236 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 4 Nov 2019 00:47:46 +0100 Subject: [PATCH 5/7] proposal 1: expectedReplies parameter is set in insertInCommandAndReplyMap, default value stays one. overriding enableReplyInReplyMap is not necessary anymore.second proposal: the commander id is supplied in the interpretDeviceReply function, so we don't have to look for it in the DeviceCommandMap. was it removed at some point because it is listed in the documentation? --- action/ActionHelper.h | 2 +- devicehandlers/DeviceHandlerBase.cpp | 32 +++++++++++++++++--------- devicehandlers/DeviceHandlerBase.h | 16 ++++++++----- modes/ModeHelper.h | 13 ++++++++++- tmtcservices/CommandingServiceBase.cpp | 11 ++++----- 5 files changed, 49 insertions(+), 25 deletions(-) diff --git a/action/ActionHelper.h b/action/ActionHelper.h index 6ba6dd89..a72e3654 100644 --- a/action/ActionHelper.h +++ b/action/ActionHelper.h @@ -74,7 +74,7 @@ public: protected: static const uint8_t STEP_OFFSET = 1;//!< Increase of value of this per step HasActionsIF* owner;//!< Pointer to the owner - MessageQueueIF* queueToUse;//!< Queue to be used as response sender, has to be set with + MessageQueueIF* queueToUse;//!< Queue to be used as response sender, has to be set with @c setQueueToUse StorageManagerIF* ipcStore;//!< Pointer to an IPC Store, initialized during construction or initialize(MessageQueueIF* queueToUse_) or with setQueueToUse(MessageQueueIF *queue) /** *Internal function called by handleActionMessage(CommandMessage* command) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 1bb411b7..c92cac33 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -255,9 +255,9 @@ ReturnValue_t DeviceHandlerBase::isModeCombinationValid(Mode_t mode, ReturnValue_t DeviceHandlerBase::insertInCommandAndReplyMap( DeviceCommandId_t deviceCommand, uint16_t maxDelayCycles, - uint8_t periodic, bool hasDifferentReplyId, DeviceCommandId_t replyId) { -//No need to check, as we may try to insert multiple times. - insertInCommandMap(deviceCommand); + uint8_t periodic, uint8_t expectedReplies, bool hasDifferentReplyId, DeviceCommandId_t replyId) { + //No need to check, as we may try to insert multiple times. + insertInCommandMap(deviceCommand,expectedReplies); if (hasDifferentReplyId) { return insertInReplyMap(replyId, maxDelayCycles, periodic); } else { @@ -283,9 +283,9 @@ ReturnValue_t DeviceHandlerBase::insertInReplyMap(DeviceCommandId_t replyId, } ReturnValue_t DeviceHandlerBase::insertInCommandMap( - DeviceCommandId_t deviceCommand) { + DeviceCommandId_t deviceCommand, uint8_t expectedReplies) { DeviceCommandInfo info; - info.expectedReplies = 0; + info.expectedReplies = expectedReplies; info.isExecuting = false; info.sendReplyTo = NO_COMMANDER; std::pair::iterator, bool> returnValue; @@ -712,6 +712,7 @@ void DeviceHandlerBase::handleReply(const uint8_t* receivedData, DeviceCommandId_t foundId, uint32_t foundLen) { ReturnValue_t result; DeviceReplyMap::iterator iter = deviceReplyMap.find(foundId); + MessageQueueId_t commander; if (iter == deviceReplyMap.end()) { replyRawReplyIfnotWiretapped(receivedData, foundLen); @@ -728,7 +729,17 @@ void DeviceHandlerBase::handleReply(const uint8_t* receivedData, } else { info->delayCycles = 0; } - result = interpretDeviceReply(foundId, receivedData); + + DeviceCommandMap::iterator commandIter = deviceCommandMap.find(foundId); + // could be a reply only packet + if(commandIter == deviceCommandMap.end()) { + commander = 0; + } + else { + commander = commandIter->second.sendReplyTo; + } + + result = interpretDeviceReply(foundId, receivedData,commander); if (result != RETURN_OK) { //Report failed interpretation to FDIR. replyRawReplyIfnotWiretapped(receivedData, foundLen); @@ -798,7 +809,7 @@ void DeviceHandlerBase::modeChanged(void) { } ReturnValue_t DeviceHandlerBase::enableReplyInReplyMap( - DeviceCommandMap::iterator command, uint8_t expectedReplies, + DeviceCommandMap::iterator command,/* uint8_t expectedReplies, */ bool useAlternativeId, DeviceCommandId_t alternativeReply) { DeviceReplyMap::iterator iter; if (useAlternativeId) { @@ -810,7 +821,7 @@ ReturnValue_t DeviceHandlerBase::enableReplyInReplyMap( DeviceReplyInfo *info = &(iter->second); info->delayCycles = info->maxDelayCycles; info->command = command; - command->second.expectedReplies = expectedReplies; + // command->second.expectedReplies = expectedReplies; return RETURN_OK; } else { return NO_REPLY_EXPECTED; @@ -1108,8 +1119,7 @@ void DeviceHandlerBase::handleDeviceTM(SerializeIF* data, // hiding of sender needed so the service will handle it as unexpected Data, no matter what state //(progress or completed) it is in - actionHelper.reportData(defaultRawReceiver, replyId, &wrapper, - true); + actionHelper.reportData(defaultRawReceiver, replyId, &wrapper); } } else { //unrequested/aperiodic replies @@ -1122,7 +1132,7 @@ void DeviceHandlerBase::handleDeviceTM(SerializeIF* data, true); } } -//Try to cast to DataSet and commit data. + //Try to cast to DataSet and commit data. if (!neverInDataPool) { DataSet* dataSet = dynamic_cast(data); if (dataSet != NULL) { diff --git a/devicehandlers/DeviceHandlerBase.h b/devicehandlers/DeviceHandlerBase.h index be488d35..89f96503 100644 --- a/devicehandlers/DeviceHandlerBase.h +++ b/devicehandlers/DeviceHandlerBase.h @@ -576,8 +576,9 @@ protected: * Default is aperiodic (0) * @return RETURN_OK when the command was successfully inserted, COMMAND_MAP_ERROR else. */ + // Proposal: Set expected replies for a command in insertInCommandAndReplyMap so we don't have to overwrite enableReplyInReplyMap ReturnValue_t insertInCommandAndReplyMap(DeviceCommandId_t deviceCommand, - uint16_t maxDelayCycles, uint8_t periodic = 0, + uint16_t maxDelayCycles, uint8_t periodic = 0, uint8_t expectedReplies = 1, bool hasDifferentReplyId = false, DeviceCommandId_t replyId = 0); /** * This is a helper method to insert replies in the reply map. @@ -594,7 +595,7 @@ protected: * @param deviceCommand The command to add * @return RETURN_OK if the command was successfully inserted, RETURN_FAILED else. */ - ReturnValue_t insertInCommandMap(DeviceCommandId_t deviceCommand); + ReturnValue_t insertInCommandMap(DeviceCommandId_t deviceCommand, uint8_t expectedReplies = 1); /** * This is a helper method to facilitate updating entries in the reply map. * @param deviceCommand Identifier of the reply to update. @@ -644,18 +645,19 @@ protected: * * This is called after scanForReply() found a valid packet, it can be assumed that the length and structure is valid. * This routine extracts the data from the packet into a DataSet and then calls handleDeviceTM(), which either sends - * a TM packet or stores the data in the DataPool depending on whether the it was an external command. + * a TM packet or stores the data in the DataPool depending on whether it was an external command. * No packet length is given, as it should be defined implicitly by the id. * * @param id the id found by scanForReply() * @param packet - * @param commander the one who initiated the command, is 0 if not external commanded + * @param commander the one who initiated the command, is 0 if not external commanded. + * UPDATE: This parameter does not exist anymore. Why? * @return * - @c RETURN_OK when the reply was interpreted. * - @c RETURN_FAILED when the reply could not be interpreted, eg. logical errors or range violations occurred */ virtual ReturnValue_t interpretDeviceReply(DeviceCommandId_t id, - const uint8_t *packet) = 0; + const uint8_t *packet, MessageQueueId_t commander = 0) = 0; /** * Construct a command reply containing a raw reply. @@ -723,13 +725,15 @@ protected: * - If the command was not found in the reply map, NO_REPLY_EXPECTED MUST be returned. * - A failure code may be returned if something went fundamentally wrong. * + * * @param deviceCommand * @return - RETURN_OK if a reply was activated. * - NO_REPLY_EXPECTED if there was no reply found. This is not an error case as many commands * do not expect a reply. */ + // Proposal: Set expected replies in insertInCommandAndReplyMap so we don't have to overwrite that function anymore. virtual ReturnValue_t enableReplyInReplyMap(DeviceCommandMap::iterator cmd, - uint8_t expectedReplies = 1, bool useAlternateId = false, + /* uint8_t expectedReplies = 0 */ bool useAlternateId = false, DeviceCommandId_t alternateReplyID = 0); /** diff --git a/modes/ModeHelper.h b/modes/ModeHelper.h index 87f6d411..b23eb69e 100644 --- a/modes/ModeHelper.h +++ b/modes/ModeHelper.h @@ -16,11 +16,17 @@ public: ModeHelper(HasModesIF *owner); virtual ~ModeHelper(); + /** + * This is used by DHB to handle all mode messages issued by a service + * @param message + * @return + */ ReturnValue_t handleModeCommand(CommandMessage *message); /** * - * @param parentQueue the Queue id of the parent object. Set to 0 if no parent present + * @param parentQueue the Queue id of the parent object (assembly or subsystem object). + * Set to 0 if no parent present */ void setParentQueue(MessageQueueId_t parentQueueId); @@ -28,6 +34,11 @@ public: ReturnValue_t initialize(void); //void is there to stop eclipse CODAN from falsely reporting an error + /** + * Used to notify + * @param mode + * @param submode + */ void modeChanged(Mode_t mode, Submode_t submode); void startTimer(uint32_t timeoutMs); diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index d70b9042..ec65492d 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -11,12 +11,11 @@ CommandingServiceBase::CommandingServiceBase(object_id_t setObjectId, uint16_t apid, uint8_t service, uint8_t numberOfParallelCommands, uint16_t commandTimeout_seconds, object_id_t setPacketSource, object_id_t setPacketDestination, size_t queueDepth) : - SystemObject(setObjectId), apid(apid), service(service), timeout_seconds( - commandTimeout_seconds), tmPacketCounter(0), IPCStore(NULL), TCStore( - NULL), commandQueue(NULL), requestQueue(NULL), commandMap( - numberOfParallelCommands), failureParameter1(0), failureParameter2( - 0), packetSource(setPacketSource), packetDestination( - setPacketDestination),executingTask(NULL) { + SystemObject(setObjectId), apid(apid), service(service), + timeout_seconds(commandTimeout_seconds), tmPacketCounter(0), IPCStore(NULL), + TCStore(NULL), commandQueue(NULL), requestQueue(NULL), commandMap(numberOfParallelCommands), + failureParameter1(0), failureParameter2(0), packetSource(setPacketSource), + packetDestination(setPacketDestination),executingTask(NULL) { commandQueue = QueueFactory::instance()->createMessageQueue(queueDepth); requestQueue = QueueFactory::instance()->createMessageQueue(queueDepth); } From 12f51575eb42bffc9e2cf5a72a5678975f87db34 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 4 Nov 2019 00:53:05 +0100 Subject: [PATCH 6/7] removed a flag by accident, fixed now --- devicehandlers/DeviceHandlerBase.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index c92cac33..3018a076 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -1119,7 +1119,7 @@ void DeviceHandlerBase::handleDeviceTM(SerializeIF* data, // hiding of sender needed so the service will handle it as unexpected Data, no matter what state //(progress or completed) it is in - actionHelper.reportData(defaultRawReceiver, replyId, &wrapper); + actionHelper.reportData(defaultRawReceiver, replyId, &wrapper, true); } } else { //unrequested/aperiodic replies From cb919ada2a99b818624d05a4e4fdfeb85e9b8174 Mon Sep 17 00:00:00 2001 From: "Robin.Mueller" Date: Mon, 4 Nov 2019 01:55:40 +0100 Subject: [PATCH 7/7] assuming that a default value of 0 for expectedReplies is needed, I introduced a new variable into DeviceCommandInfo, which stores another number of replies expected. this value is assigned in enableReplyInReplyMap. That way, the initial value of 0 remains the same (if it was needed), and is only set to another desired value if a write was sent --- devicehandlers/DeviceHandlerBase.cpp | 5 +++-- devicehandlers/DeviceHandlerBase.h | 9 +++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/devicehandlers/DeviceHandlerBase.cpp b/devicehandlers/DeviceHandlerBase.cpp index 3018a076..491d792b 100644 --- a/devicehandlers/DeviceHandlerBase.cpp +++ b/devicehandlers/DeviceHandlerBase.cpp @@ -285,7 +285,8 @@ ReturnValue_t DeviceHandlerBase::insertInReplyMap(DeviceCommandId_t replyId, ReturnValue_t DeviceHandlerBase::insertInCommandMap( DeviceCommandId_t deviceCommand, uint8_t expectedReplies) { DeviceCommandInfo info; - info.expectedReplies = expectedReplies; + info.expectedReplies = 0; + info.expectedRepliesWhenEnablingReplyMap = expectedReplies; info.isExecuting = false; info.sendReplyTo = NO_COMMANDER; std::pair::iterator, bool> returnValue; @@ -821,7 +822,7 @@ ReturnValue_t DeviceHandlerBase::enableReplyInReplyMap( DeviceReplyInfo *info = &(iter->second); info->delayCycles = info->maxDelayCycles; info->command = command; - // command->second.expectedReplies = expectedReplies; + command->second.expectedReplies = command->second.expectedRepliesWhenEnablingReplyMap; return RETURN_OK; } else { return NO_REPLY_EXPECTED; diff --git a/devicehandlers/DeviceHandlerBase.h b/devicehandlers/DeviceHandlerBase.h index 89f96503..ad4a2203 100644 --- a/devicehandlers/DeviceHandlerBase.h +++ b/devicehandlers/DeviceHandlerBase.h @@ -704,7 +704,8 @@ protected: struct DeviceCommandInfo { bool isExecuting; //!< Indicates if the command is already executing. - uint8_t expectedReplies; //!< Dynamic value to indicate how many replies are expected. + uint8_t expectedReplies; //!< Dynamic value to indicate how many replies are expected. Inititated with 0. + uint8_t expectedRepliesWhenEnablingReplyMap; //!< Constant value which specifies expected replies when enabling reply map. Inititated in insertInCommandAndReplyMap() MessageQueueId_t sendReplyTo; //!< if this is != NO_COMMANDER, DHB was commanded externally and shall report everything to commander. }; @@ -732,8 +733,12 @@ protected: * do not expect a reply. */ // Proposal: Set expected replies in insertInCommandAndReplyMap so we don't have to overwrite that function anymore. + // Replies are only checked when a write was issued and the default value here was one, so + // it should be possible to set this in the DeviceCommandMap with default value one. + // UPDATE: The default value of 0 when inserting into command and reply map is retained now by introducing a new + // variable in the DeviceCommandInfo which specifies expected replies if this function is called. virtual ReturnValue_t enableReplyInReplyMap(DeviceCommandMap::iterator cmd, - /* uint8_t expectedReplies = 0 */ bool useAlternateId = false, + /* uint8_t expectedReplies = 1, */ bool useAlternateId = false, DeviceCommandId_t alternateReplyID = 0); /**