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?

This commit is contained in:
Robin Müller 2019-11-04 00:47:46 +01:00
parent 46986f69e4
commit 8eb1a5b13e
5 changed files with 49 additions and 25 deletions

View File

@ -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)

View File

@ -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<std::map<DeviceCommandId_t, DeviceCommandInfo>::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<DataSet*>(data);
if (dataSet != NULL) {

View File

@ -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);
/**

View File

@ -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);

View File

@ -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);
}