Skip to content

Commit

Permalink
Fix nasa#63, remove macros within C code
Browse files Browse the repository at this point in the history 
Reworks the CF_CmdGetSetParam to be clearer in its
implementation, not require the use of endian-specific
conditionally-compiled code.
  • Loading branch information
@jphickey
jphickey committed Jan 11, 2022
1 parent c0b1f53 commit cefc41e
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 64 deletions.
150 changes: 103 additions & 47 deletions fsw/src/cf_cmd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -911,43 +911,72 @@ int CF_CmdValidateMaxOutgoing(uint32 val, uint8 chan_num)
*************************************************************************/
/* combine getset into a single function with a branch to avoid wasted memory footprint with duplicate
* logic for finding the parameter */
void CF_CmdGetSetParam(uint8 is_set, uint8 param_id, uint32 value, uint8 chan_num)
void CF_CmdGetSetParam(uint8 is_set, CF_GetSet_ValueID_t param_id, uint32 value, uint8 chan_num)
{
/* These macros define entries into the paramater array. The mapping of the array is
* ground parameter to configuration parameter. This logic allows a simple access
* of the configuration parameter or a check on validity of the parameter and then
* access. */
#define SPTR(x) \
{ \
&CF_AppData.config_table->x, sizeof(CF_AppData.config_table->x), NULL \
}
#define SPTRFN(x, fn) \
{ \
&CF_AppData.config_table->x, sizeof(CF_AppData.config_table->x), fn \
}
#define CPTRFN(x, fn) \
{ \
&CF_AppData.config_table->chan[chan_num].x, sizeof(CF_AppData.config_table->chan[chan_num].x), fn \
}
const struct
CF_ConfigTable_t *config;
int acc;
bool valid_set;
struct
{
void *ptr;
uint32 size;
int (*fn)(uint32, uint8 chan_num);
} items[CF_NUM_CFG_PACKET_ITEMS] = {
SPTR(ticks_per_second), SPTR(rx_crc_calc_bytes_per_wakeup),
SPTR(ack_timer_s), SPTR(nak_timer_s),
SPTR(inactivity_timer_s), SPTRFN(outgoing_file_chunk_size, CF_CmdValidateChunkSize),
SPTR(ack_limit), SPTR(nak_limit),
SPTR(local_eid), CPTRFN(max_outgoing_messages_per_wakeup, CF_CmdValidateMaxOutgoing)};
} item;

int acc = 1; /* 1 means to reject */
acc = 1; /* 1 means to reject */
valid_set = false;
config = CF_AppData.config_table;
memset(&item, 0, sizeof(item));

#if ENDIAN == _EB
static const int shift_map[5] = {0, 24, 16, 8, 0};
#endif
switch (param_id)
{
case CF_GetSet_ValueID_ticks_per_second:
item.ptr = &config->ticks_per_second;
item.size = sizeof(config->ticks_per_second);
break;
case CF_GetSet_ValueID_rx_crc_calc_bytes_per_wakeup:
item.ptr = &config->rx_crc_calc_bytes_per_wakeup;
item.size = sizeof(config->rx_crc_calc_bytes_per_wakeup);
break;
case CF_GetSet_ValueID_ack_timer_s:
item.ptr = &config->ack_timer_s;
item.size = sizeof(config->ack_timer_s);
break;
case CF_GetSet_ValueID_nak_timer_s:
item.ptr = &config->nak_timer_s;
item.size = sizeof(config->nak_timer_s);
break;
case CF_GetSet_ValueID_inactivity_timer_s:
item.ptr = &config->inactivity_timer_s;
item.size = sizeof(config->inactivity_timer_s);
break;
case CF_GetSet_ValueID_outgoing_file_chunk_size:
item.ptr = &config->outgoing_file_chunk_size;
item.size = sizeof(config->outgoing_file_chunk_size);
item.fn = CF_CmdValidateChunkSize;
break;
case CF_GetSet_ValueID_ack_limit:
item.ptr = &config->ack_limit;
item.size = sizeof(config->ack_limit);
break;
case CF_GetSet_ValueID_nak_limit:
item.ptr = &config->nak_limit;
item.size = sizeof(config->nak_limit);
break;
case CF_GetSet_ValueID_local_eid:
item.ptr = &config->local_eid;
item.size = sizeof(config->local_eid);
break;
case CF_GetSet_ValueID_chan_max_outgoing_messages_per_wakeup:
item.ptr = &config->chan[chan_num].max_outgoing_messages_per_wakeup;
item.size = sizeof(config->chan[chan_num].max_outgoing_messages_per_wakeup);
item.fn = CF_CmdValidateMaxOutgoing;
break;
default:
break;
};

if (param_id >= CF_NUM_CFG_PACKET_ITEMS)
if (item.size == 0)
{
CFE_EVS_SendEvent(CF_EID_ERR_CMD_GETSET_PARAM, CFE_EVS_EventType_ERROR,
"CF: invalid configuration parameter id %d received", param_id);
Expand All @@ -963,11 +992,9 @@ void CF_CmdGetSetParam(uint8 is_set, uint8 param_id, uint32 value, uint8 chan_nu

if (is_set)
{
int valid_set = 0;

if (items[param_id].fn)
if (item.fn)
{
if (!items[param_id].fn(value, chan_num))
if (!item.fn(value, chan_num))
{
valid_set = 1;
}
Expand All @@ -984,26 +1011,55 @@ void CF_CmdGetSetParam(uint8 is_set, uint8 param_id, uint32 value, uint8 chan_nu

if (valid_set)
{
CFE_EVS_SendEvent(CF_EID_INF_CMD_GETSET1, CFE_EVS_EventType_INFORMATION,
"CF: setting parameter id %d to %d", param_id, value);
#if ENDIAN == _EB
CF_Assert((items[param_id].size > 0) && (items[param_id].size < 5));
value <<= shift_map[items[param_id].size];
#endif
memcpy(items[param_id].ptr, &value, items[param_id].size);
acc = 0;

CFE_EVS_SendEvent(CF_EID_INF_CMD_GETSET1, CFE_EVS_EventType_INFORMATION,
"CF: setting parameter id %d to %lu", param_id, (unsigned long)value);

/* Store value based on its size */
if (item.size == sizeof(uint32))
{
*((uint32 *)item.ptr) = value;
}
else if (item.size == sizeof(uint16))
{
*((uint16 *)item.ptr) = value;
}
else if (item.size == sizeof(uint8))
{
*((uint8 *)item.ptr) = value;
}
else
{
/* unimplemented store; this is a SW configuration error! */
acc = 1;
}
}
}
else
{
memcpy(&value, items[param_id].ptr, items[param_id].size);
#if ENDIAN == _EB
CF_Assert((items[param_id].size > 0) && (items[param_id].size < 5));
value >>= shift_map[items[param_id].size];
#endif
CFE_EVS_SendEvent(CF_EID_INF_CMD_GETSET2, CFE_EVS_EventType_INFORMATION, "CF: parameter id %d = %d", param_id,
value);
acc = 0;

/* Read value depending on its size */
if (item.size == sizeof(uint32))
{
value = *((const uint32 *)item.ptr);
}
else if (item.size == sizeof(uint16))
{
value = *((const uint16 *)item.ptr);
}
else if (item.size == sizeof(uint8))
{
value = *((const uint8 *)item.ptr);
}
else
{
acc = 1;
}

CFE_EVS_SendEvent(CF_EID_INF_CMD_GETSET2, CFE_EVS_EventType_INFORMATION, "CF: parameter id %d = %lu", param_id,
(unsigned long)value);
}

err_out:
Expand Down
2 changes: 1 addition & 1 deletion fsw/src/cf_cmd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ void CF_CmdWriteQueue(CFE_SB_Buffer_t *msg);
void CF_CmdSendCfgParams(CFE_SB_Buffer_t *msg);
int CF_CmdValidateChunkSize(uint32 val, uint8 chan_num /* ignored */);
int CF_CmdValidateMaxOutgoing(uint32 val, uint8 chan_num);
void CF_CmdGetSetParam(uint8 is_set, uint8 param_id, uint32 value, uint8 chan_num);
void CF_CmdGetSetParam(uint8 is_set, CF_GetSet_ValueID_t param_id, uint32 value, uint8 chan_num);
void CF_CmdSetParam(CFE_SB_Buffer_t *msg);
void CF_CmdGetParam(CFE_SB_Buffer_t *msg);
void CF_CmdEnableEngine(CFE_SB_Buffer_t *msg);
Expand Down
25 changes: 23 additions & 2 deletions fsw/src/cf_msg.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,7 @@ typedef struct CF_ConfigPacket
uint8 nak_limit; /* number of times to retry NAK before giving up (resets on a single response */

CF_EntityId_t local_eid;
/* must #define the number of data items in this struct for command processing */
#define CF_NUM_CFG_PACKET_ITEMS 10

} CF_ConfigPacket_t;

/****************************************
Expand Down Expand Up @@ -183,6 +182,27 @@ typedef struct
CF_UnionArgs_Payload_t data;
} CF_UnionArgsCmd_t;

/**
* @brief Parameter IDs for use with Get/Set param messages
*
* Specifically these are used for the "key" field within CF_GetParamCmd_t and
* CF_SetParamCmd_t message structures.
*/
typedef enum
{
CF_GetSet_ValueID_ticks_per_second,
CF_GetSet_ValueID_rx_crc_calc_bytes_per_wakeup,
CF_GetSet_ValueID_ack_timer_s,
CF_GetSet_ValueID_nak_timer_s,
CF_GetSet_ValueID_inactivity_timer_s,
CF_GetSet_ValueID_outgoing_file_chunk_size,
CF_GetSet_ValueID_ack_limit,
CF_GetSet_ValueID_nak_limit,
CF_GetSet_ValueID_local_eid,
CF_GetSet_ValueID_chan_max_outgoing_messages_per_wakeup,
CF_GetSet_ValueID_MAX
} CF_GetSet_ValueID_t;

typedef struct CF_GetParamCmd
{
CFE_MSG_CommandHeader_t cmd_header;
Expand Down Expand Up @@ -230,4 +250,5 @@ typedef struct CF_TransactionCmd
uint8 chan; /* if 254, use ts. if 255, all channels */
} CF_TransactionCmd_t;


#endif /* !CF_MSG_H */
24 changes: 12 additions & 12 deletions unit-test/cf_cmd_tests.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4157,12 +4157,12 @@ void Test_CF_CmdValidateMaxOutgoing_WhenGiven_val_Is_0_And_sem_name_Is_NULL_Retu
**
*******************************************************************************/

void Test_CF_CmdGetSetParam_When_param_id_Eq_CF_NUM_CFG_PACKET_ITEMS_FailSendEventAndRejectCmd(void)
void Test_CF_CmdGetSetParam_When_param_id_Eq_CF_GetSet_ValueID_MAX_FailSendEventAndRejectCmd(void)
{
/* Arrange */
CF_ConfigTable_t dummy_config_table;
uint8 arg_is_set = Any_uint8();
uint8 arg_param_id = CF_NUM_CFG_PACKET_ITEMS;
uint8 arg_param_id = CF_GetSet_ValueID_MAX;
uint32 arg_value = Any_uint32();
uint8 arg_chan_num = Any_uint8();

Expand All @@ -4186,14 +4186,14 @@ void Test_CF_CmdGetSetParam_When_param_id_Eq_CF_NUM_CFG_PACKET_ITEMS_FailSendEve
"CF_AppData.hk.counters.err is %d and should be 1 more than %d", CF_AppData.hk.counters.err,
initial_hk_err_counter);

} /* end Test_CF_CmdGetSetParam_When_param_id_Eq_CF_NUM_CFG_PACKET_ITEMS_FailSendEventAndRejectCmd */
} /* end Test_CF_CmdGetSetParam_When_param_id_Eq_CF_GetSet_ValueID_MAX_FailSendEventAndRejectCmd */

void Test_CF_CmdGetSetParam_When_param_id_AnyGreaterThan_CF_NUM_CFG_PACKET_ITEMS_FailSendEventAndRejectCmd(void)
void Test_CF_CmdGetSetParam_When_param_id_AnyGreaterThan_CF_GetSet_ValueID_MAX_FailSendEventAndRejectCmd(void)
{
/* Arrange */
CF_ConfigTable_t dummy_config_table;
uint8 arg_is_set = Any_uint8();
uint8 arg_param_id = Any_uint8_GreaterThan(CF_NUM_CFG_PACKET_ITEMS);
uint8 arg_param_id = Any_uint8_GreaterThan(CF_GetSet_ValueID_MAX);
uint32 arg_value = Any_uint32();
uint8 arg_chan_num = Any_uint8();

Expand All @@ -4217,14 +4217,14 @@ void Test_CF_CmdGetSetParam_When_param_id_AnyGreaterThan_CF_NUM_CFG_PACKET_ITEMS
"CF_AppData.hk.counters.err is %u and should be 1 more than %u", CF_AppData.hk.counters.err,
initial_hk_err_counter);

} /* end Test_CF_CmdGetSetParam_When_param_id_AnyGreaterThan_CF_NUM_CFG_PACKET_ITEMS_FailSendEventAndRejectCmd */
} /* end Test_CF_CmdGetSetParam_When_param_id_AnyGreaterThan_CF_GetSet_ValueID_MAX_FailSendEventAndRejectCmd */

void Test_CF_CmdGetSetParam_Given_chan_num_IsEqTo_CF_NUM_CHANNELS_ErrorOutAndCountError(void)
{
/* Arrange */
CF_ConfigTable_t dummy_config_table;
uint8 arg_is_set = Any_uint8();
uint8 arg_param_id = Any_uint8_LessThan(CF_NUM_CFG_PACKET_ITEMS);
uint8 arg_param_id = Any_uint8_LessThan(CF_GetSet_ValueID_MAX);
uint32 arg_value = Any_uint32();
uint8 arg_chan_num = CF_NUM_CHANNELS;

Expand Down Expand Up @@ -4255,7 +4255,7 @@ void Test_CF_CmdGetSetParam_Given_chan_num_IsGreaterThan_CF_NUM_CHANNELS_ErrorOu
/* Arrange */
CF_ConfigTable_t dummy_config_table;
uint8 arg_is_set = Any_uint8();
uint8 arg_param_id = Any_uint8_LessThan(CF_NUM_CFG_PACKET_ITEMS);
uint8 arg_param_id = Any_uint8_LessThan(CF_GetSet_ValueID_MAX);
uint32 arg_value = Any_uint32();
uint8 arg_chan_num = Any_uint8_GreaterThan(CF_NUM_CHANNELS);

Expand Down Expand Up @@ -5344,12 +5344,12 @@ void add_CF_CmdValidateMaxOutgoing_tests(void)

void add_CF_CmdGetSetParam_tests(void)
{
UtTest_Add(Test_CF_CmdGetSetParam_When_param_id_Eq_CF_NUM_CFG_PACKET_ITEMS_FailSendEventAndRejectCmd,
UtTest_Add(Test_CF_CmdGetSetParam_When_param_id_Eq_CF_GetSet_ValueID_MAX_FailSendEventAndRejectCmd,
cf_cmd_tests_Setup, cf_cmd_tests_Teardown,
"Test_CF_CmdGetSetParam_When_param_id_Eq_CF_NUM_CFG_PACKET_ITEMS_FailSendEventAndRejectCmd");
UtTest_Add(Test_CF_CmdGetSetParam_When_param_id_AnyGreaterThan_CF_NUM_CFG_PACKET_ITEMS_FailSendEventAndRejectCmd,
"Test_CF_CmdGetSetParam_When_param_id_Eq_CF_GetSet_ValueID_MAX_FailSendEventAndRejectCmd");
UtTest_Add(Test_CF_CmdGetSetParam_When_param_id_AnyGreaterThan_CF_GetSet_ValueID_MAX_FailSendEventAndRejectCmd,
cf_cmd_tests_Setup, cf_cmd_tests_Teardown,
"Test_CF_CmdGetSetParam_When_param_id_AnyGreaterThan_CF_NUM_CFG_PACKET_ITEMS_FailSendEventAndRejectCmd");
"Test_CF_CmdGetSetParam_When_param_id_AnyGreaterThan_CF_GetSet_ValueID_MAX_FailSendEventAndRejectCmd");
UtTest_Add(Test_CF_CmdGetSetParam_Given_chan_num_IsEqTo_CF_NUM_CHANNELS_ErrorOutAndCountError, cf_cmd_tests_Setup,
cf_cmd_tests_Teardown,
"Test_CF_CmdGetSetParam_Given_chan_num_IsEqTo_CF_NUM_CHANNELS_ErrorOutAndCountError");
Expand Down
4 changes: 2 additions & 2 deletions unit-test/stubs/cf_cmd_stubs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,10 @@ void CF_CmdGetParam(CFE_SB_Buffer_t *msg)
* Generated stub function for CF_CmdGetSetParam()
* ----------------------------------------------------
*/
void CF_CmdGetSetParam(uint8 is_set, uint8 param_id, uint32 value, uint8 chan_num)
void CF_CmdGetSetParam(uint8 is_set, CF_GetSet_ValueID_t param_id, uint32 value, uint8 chan_num)
{
UT_GenStub_AddParam(CF_CmdGetSetParam, uint8, is_set);
UT_GenStub_AddParam(CF_CmdGetSetParam, uint8, param_id);
UT_GenStub_AddParam(CF_CmdGetSetParam, CF_GetSet_ValueID_t, param_id);
UT_GenStub_AddParam(CF_CmdGetSetParam, uint32, value);
UT_GenStub_AddParam(CF_CmdGetSetParam, uint8, chan_num);

Expand Down

0 comments on commit cefc41e

Please sign in to comment.