mgmt/osdp: Refactor struct osdp_cmd members for readability

Some of the names used in `struct osdp_cmd` where directly as in the
specification. Initially it appealed to keep them like that but with
time, a little more consistent naming of members helps if you haven't
read the specification document very recently.

Signed-off-by: Siddharth Chandrasekaran <siddharth@embedjournal.com>
This commit is contained in:
Siddharth Chandrasekaran 2020-09-03 18:25:48 +05:30 committed by Carles Cufí
parent 4c9b0ae928
commit ee99c34fb6
4 changed files with 85 additions and 74 deletions

View file

@ -17,6 +17,7 @@ extern "C" {
#endif #endif
#define OSDP_CMD_TEXT_MAX_LEN 32 #define OSDP_CMD_TEXT_MAX_LEN 32
#define OSDP_CMD_KEYSET_KEY_MAX_LEN 32
/** /**
* @brief Various card formats that a PD can support. This is sent to CP * @brief Various card formats that a PD can support. This is sent to CP
@ -41,12 +42,12 @@ enum osdp_card_formats_e {
* 4 - set the permanent state to ON, allow timed operation to complete * 4 - set the permanent state to ON, allow timed operation to complete
* 5 - set the temporary state to ON, resume perm state on timeout * 5 - set the temporary state to ON, resume perm state on timeout
* 6 - set the temporary state to OFF, resume permanent state on timeout * 6 - set the temporary state to OFF, resume permanent state on timeout
* @param tmr_count Time in units of 100 ms * @param timer_count Time in units of 100 ms
*/ */
struct osdp_cmd_output { struct osdp_cmd_output {
uint8_t output_no; uint8_t output_no;
uint8_t control_code; uint8_t control_code;
uint16_t tmr_count; uint16_t timer_count;
}; };
/** /**
@ -77,7 +78,7 @@ enum osdp_led_color_e {
* @param off_count The OFF duration of the flash, in units of 100 ms * @param off_count The OFF duration of the flash, in units of 100 ms
* @param on_color Color to set during the ON timer (enum osdp_led_color_e) * @param on_color Color to set during the ON timer (enum osdp_led_color_e)
* @param off_color Color to set during the OFF timer (enum osdp_led_color_e) * @param off_color Color to set during the OFF timer (enum osdp_led_color_e)
* @param timer Time in units of 100 ms (only for temporary mode) * @param timer_count Time in units of 100 ms (only for temporary mode)
*/ */
struct osdp_cmd_led_params { struct osdp_cmd_led_params {
uint8_t control_code; uint8_t control_code;
@ -85,7 +86,7 @@ struct osdp_cmd_led_params {
uint8_t off_count; uint8_t off_count;
uint8_t on_color; uint8_t on_color;
uint8_t off_color; uint8_t off_color;
uint16_t timer; uint16_t timer_count;
}; };
/** /**
@ -107,14 +108,14 @@ struct osdp_cmd_led {
* @brief Sent from CP to control the behaviour of a buzzer in the PD. * @brief Sent from CP to control the behaviour of a buzzer in the PD.
* *
* @param reader 0 = First Reader, 1 = Second Reader, etc. * @param reader 0 = First Reader, 1 = Second Reader, etc.
* @param tone_code 0: no tone, 1: off, 2: default tone, 3+ is TBD. * @param control_code 0: no tone, 1: off, 2: default tone, 3+ is TBD.
* @param on_count The ON duration of the flash, in units of 100 ms * @param on_count The ON duration of the flash, in units of 100 ms
* @param off_count The OFF duration of the flash, in units of 100 ms * @param off_count The OFF duration of the flash, in units of 100 ms
* @param rep_count The number of times to repeat the ON/OFF cycle; 0: forever * @param rep_count The number of times to repeat the ON/OFF cycle; 0: forever
*/ */
struct osdp_cmd_buzzer { struct osdp_cmd_buzzer {
uint8_t reader; uint8_t reader;
uint8_t tone_code; uint8_t control_code;
uint8_t on_count; uint8_t on_count;
uint8_t off_count; uint8_t off_count;
uint8_t rep_count; uint8_t rep_count;
@ -124,7 +125,7 @@ struct osdp_cmd_buzzer {
* @brief Command to manuplate any display units that the PD supports. * @brief Command to manuplate any display units that the PD supports.
* *
* @param reader 0 = First Reader, 1 = Second Reader, etc. * @param reader 0 = First Reader, 1 = Second Reader, etc.
* @param cmd One of the following: * @param control_code One of the following:
* 1 - permanent text, no wrap * 1 - permanent text, no wrap
* 2 - permanent text, with wrap * 2 - permanent text, with wrap
* 3 - temp text, no wrap * 3 - temp text, no wrap
@ -137,7 +138,7 @@ struct osdp_cmd_buzzer {
*/ */
struct osdp_cmd_text { struct osdp_cmd_text {
uint8_t reader; uint8_t reader;
uint8_t cmd; uint8_t control_code;
uint8_t temp_time; uint8_t temp_time;
uint8_t offset_row; uint8_t offset_row;
uint8_t offset_col; uint8_t offset_col;
@ -149,27 +150,27 @@ struct osdp_cmd_text {
* @brief Sent in response to a COMSET command. Set communication parameters to * @brief Sent in response to a COMSET command. Set communication parameters to
* PD. Must be stored in PD non-volatile memory. * PD. Must be stored in PD non-volatile memory.
* *
* @param addr Unit ID to which this PD will respond after the change takes * @param address Unit ID to which this PD will respond after the change takes
* effect. * effect.
* @param baud baud rate value 9600/38400/115200 * @param baud_rate baud rate value 9600/38400/115200
*/ */
struct osdp_cmd_comset { struct osdp_cmd_comset {
uint8_t addr; uint8_t address;
uint32_t baud; uint32_t baud_rate;
}; };
/** /**
* @brief This command transfers an encryption key from the CP to a PD. * @brief This command transfers an encryption key from the CP to a PD.
* *
* @param key_type Type of keys: * @param type Type of keys:
* - 0x01 Secure Channel Base Key * - 0x01 Secure Channel Base Key
* @param len Number of bytes of key data - (Key Length in bits + 7) / 8 * @param length Number of bytes of key data - (Key Length in bits + 7) / 8
* @param data Key data * @param data Key data
*/ */
struct osdp_cmd_keyset { struct osdp_cmd_keyset {
uint8_t key_type; uint8_t type;
uint8_t len; uint8_t length;
uint8_t data[32]; uint8_t data[OSDP_CMD_KEYSET_KEY_MAX_LEN];
}; };
/** /**
@ -199,7 +200,7 @@ enum osdp_cmd_e {
*/ */
struct osdp_cmd { struct osdp_cmd {
sys_snode_t node; sys_snode_t node;
int id; enum osdp_cmd_e id;
union { union {
struct osdp_cmd_led led; struct osdp_cmd_led led;
struct osdp_cmd_buzzer buzzer; struct osdp_cmd_buzzer buzzer;
@ -213,7 +214,7 @@ struct osdp_cmd {
#ifdef CONFIG_OSDP_MODE_PD #ifdef CONFIG_OSDP_MODE_PD
/** /**
* @param cmd pointer to a command structure that would be filled by the driver. * @param cmd pointer to a command structure that was received by the driver.
* *
* @retval 0 on success. * @retval 0 on success.
* @retval -1 on failure. * @retval -1 on failure.

View file

@ -67,7 +67,7 @@ void main(void)
struct osdp_cmd_output pulse_output = { struct osdp_cmd_output pulse_output = {
.output_no = 0, /* First output */ .output_no = 0, /* First output */
.control_code = 5, /* Temporarily turn on output */ .control_code = 5, /* Temporarily turn on output */
.tmr_count = 10, /* Timer: 10 * 100ms = 1 second */ .timer_count = 10, /* Timer: 10 * 100ms = 1 second */
}; };
dev = device_get_binding(LED0); dev = device_get_binding(LED0);

View file

@ -154,8 +154,8 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len)
buf[len++] = pd->cmd_id; buf[len++] = pd->cmd_id;
buf[len++] = cmd->output.output_no; buf[len++] = cmd->output.output_no;
buf[len++] = cmd->output.control_code; buf[len++] = cmd->output.control_code;
buf[len++] = BYTE_0(cmd->output.tmr_count); buf[len++] = BYTE_0(cmd->output.timer_count);
buf[len++] = BYTE_1(cmd->output.tmr_count); buf[len++] = BYTE_1(cmd->output.timer_count);
ret = 0; ret = 0;
break; break;
case CMD_LED: case CMD_LED:
@ -172,8 +172,8 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len)
buf[len++] = cmd->led.temporary.off_count; buf[len++] = cmd->led.temporary.off_count;
buf[len++] = cmd->led.temporary.on_color; buf[len++] = cmd->led.temporary.on_color;
buf[len++] = cmd->led.temporary.off_color; buf[len++] = cmd->led.temporary.off_color;
buf[len++] = BYTE_0(cmd->led.temporary.timer); buf[len++] = BYTE_0(cmd->led.temporary.timer_count);
buf[len++] = BYTE_1(cmd->led.temporary.timer); buf[len++] = BYTE_1(cmd->led.temporary.timer_count);
buf[len++] = cmd->led.permanent.control_code; buf[len++] = cmd->led.permanent.control_code;
buf[len++] = cmd->led.permanent.on_count; buf[len++] = cmd->led.permanent.on_count;
@ -189,7 +189,7 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len)
cmd = (struct osdp_cmd *)pd->cmd_data; cmd = (struct osdp_cmd *)pd->cmd_data;
buf[len++] = pd->cmd_id; buf[len++] = pd->cmd_id;
buf[len++] = cmd->buzzer.reader; buf[len++] = cmd->buzzer.reader;
buf[len++] = cmd->buzzer.tone_code; buf[len++] = cmd->buzzer.control_code;
buf[len++] = cmd->buzzer.on_count; buf[len++] = cmd->buzzer.on_count;
buf[len++] = cmd->buzzer.off_count; buf[len++] = cmd->buzzer.off_count;
buf[len++] = cmd->buzzer.rep_count; buf[len++] = cmd->buzzer.rep_count;
@ -202,7 +202,7 @@ static int cp_build_command(struct osdp_pd *pd, uint8_t *buf, int max_len)
} }
buf[len++] = pd->cmd_id; buf[len++] = pd->cmd_id;
buf[len++] = cmd->text.reader; buf[len++] = cmd->text.reader;
buf[len++] = cmd->text.cmd; buf[len++] = cmd->text.control_code;
buf[len++] = cmd->text.temp_time; buf[len++] = cmd->text.temp_time;
buf[len++] = cmd->text.offset_row; buf[len++] = cmd->text.offset_row;
buf[len++] = cmd->text.offset_col; buf[len++] = cmd->text.offset_col;
@ -486,19 +486,6 @@ static int cp_process_reply(struct osdp_pd *pd)
return cp_decode_response(pd, pd->rx_buf, pd->rx_buf_len); return cp_decode_response(pd, pd->rx_buf, pd->rx_buf_len);
} }
static int cp_alloc_command(struct osdp_pd *pd, struct osdp_cmd **cmd)
{
void *p;
p = osdp_cmd_alloc(pd);
if (p == NULL) {
LOG_WRN(TAG "Failed to alloc cmd");
return -1;
}
*cmd = p;
return 0;
}
static void cp_flush_command_queue(struct osdp_pd *pd) static void cp_flush_command_queue(struct osdp_pd *pd)
{ {
struct osdp_cmd *cmd; struct osdp_cmd *cmd;
@ -620,7 +607,8 @@ static int cp_cmd_dispatcher(struct osdp_pd *pd, int cmd)
return 0; return 0;
} }
if (cp_alloc_command(pd, &c)) { c = osdp_cmd_alloc(pd);
if (c == NULL) {
return OSDP_CP_ERR_GENERIC; return OSDP_CP_ERR_GENERIC;
} }
@ -723,7 +711,7 @@ int osdp_cp_set_callback_card_read(
{ {
struct osdp *ctx = osdp_get_ctx(); struct osdp *ctx = osdp_get_ctx();
ctx->cp->notifier.cardread = cb; TO_CP(ctx)->notifier.cardread = cb;
return 0; return 0;
} }
@ -733,6 +721,10 @@ int osdp_cp_send_cmd_output(int pd, struct osdp_cmd_output *p)
struct osdp *ctx = osdp_get_ctx(); struct osdp *ctx = osdp_get_ctx();
struct osdp_cmd *cmd; struct osdp_cmd *cmd;
if (TO_PD(ctx, pd)->state != OSDP_CP_STATE_ONLINE) {
LOG_WRN(TAG "PD not online");
return -1;
}
if (pd < 0 || pd >= NUM_PD(ctx)) { if (pd < 0 || pd >= NUM_PD(ctx)) {
LOG_ERR(TAG "Invalid PD number"); LOG_ERR(TAG "Invalid PD number");
return -1; return -1;
@ -754,6 +746,10 @@ int osdp_cp_send_cmd_led(int pd, struct osdp_cmd_led *p)
struct osdp *ctx = osdp_get_ctx(); struct osdp *ctx = osdp_get_ctx();
struct osdp_cmd *cmd; struct osdp_cmd *cmd;
if (TO_PD(ctx, pd)->state != OSDP_CP_STATE_ONLINE) {
LOG_WRN(TAG "PD not online");
return -1;
}
if (pd < 0 || pd >= NUM_PD(ctx)) { if (pd < 0 || pd >= NUM_PD(ctx)) {
LOG_ERR(TAG "Invalid PD number"); LOG_ERR(TAG "Invalid PD number");
return -1; return -1;
@ -775,6 +771,10 @@ int osdp_cp_send_cmd_buzzer(int pd, struct osdp_cmd_buzzer *p)
struct osdp *ctx = osdp_get_ctx(); struct osdp *ctx = osdp_get_ctx();
struct osdp_cmd *cmd; struct osdp_cmd *cmd;
if (TO_PD(ctx, pd)->state != OSDP_CP_STATE_ONLINE) {
LOG_WRN(TAG "PD not online");
return -1;
}
if (pd < 0 || pd >= NUM_PD(ctx)) { if (pd < 0 || pd >= NUM_PD(ctx)) {
LOG_ERR(TAG "Invalid PD number"); LOG_ERR(TAG "Invalid PD number");
return -1; return -1;
@ -796,6 +796,10 @@ int osdp_cp_send_cmd_text(int pd, struct osdp_cmd_text *p)
struct osdp *ctx = osdp_get_ctx(); struct osdp *ctx = osdp_get_ctx();
struct osdp_cmd *cmd; struct osdp_cmd *cmd;
if (TO_PD(ctx, pd)->state != OSDP_CP_STATE_ONLINE) {
LOG_WRN(TAG "PD not online");
return -1;
}
if (pd < 0 || pd >= NUM_PD(ctx)) { if (pd < 0 || pd >= NUM_PD(ctx)) {
LOG_ERR(TAG "Invalid PD number"); LOG_ERR(TAG "Invalid PD number");
return -1; return -1;
@ -817,6 +821,10 @@ int osdp_cp_send_cmd_comset(int pd, struct osdp_cmd_comset *p)
struct osdp *ctx = osdp_get_ctx(); struct osdp *ctx = osdp_get_ctx();
struct osdp_cmd *cmd; struct osdp_cmd *cmd;
if (TO_PD(ctx, pd)->state != OSDP_CP_STATE_ONLINE) {
LOG_WRN(TAG "PD not online");
return -1;
}
if (pd < 0 || pd >= NUM_PD(ctx)) { if (pd < 0 || pd >= NUM_PD(ctx)) {
LOG_ERR(TAG "Invalid PD number"); LOG_ERR(TAG "Invalid PD number");
return -1; return -1;

View file

@ -164,8 +164,8 @@ static void pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len)
cmd->id = OSDP_CMD_OUTPUT; cmd->id = OSDP_CMD_OUTPUT;
cmd->output.output_no = buf[pos++]; cmd->output.output_no = buf[pos++];
cmd->output.control_code = buf[pos++]; cmd->output.control_code = buf[pos++];
cmd->output.tmr_count = buf[pos++]; cmd->output.timer_count = buf[pos++];
cmd->output.tmr_count |= buf[pos++] << 8; cmd->output.timer_count |= buf[pos++] << 8;
osdp_cmd_enqueue(pd, cmd); osdp_cmd_enqueue(pd, cmd);
pd->reply_id = REPLY_ACK; pd->reply_id = REPLY_ACK;
ret = 0; ret = 0;
@ -188,8 +188,8 @@ static void pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len)
cmd->led.temporary.off_count = buf[pos++]; cmd->led.temporary.off_count = buf[pos++];
cmd->led.temporary.on_color = buf[pos++]; cmd->led.temporary.on_color = buf[pos++];
cmd->led.temporary.off_color = buf[pos++]; cmd->led.temporary.off_color = buf[pos++];
cmd->led.temporary.timer = buf[pos++]; cmd->led.temporary.timer_count = buf[pos++];
cmd->led.temporary.timer |= buf[pos++] << 8; cmd->led.temporary.timer_count |= buf[pos++] << 8;
cmd->led.permanent.control_code = buf[pos++]; cmd->led.permanent.control_code = buf[pos++];
cmd->led.permanent.on_count = buf[pos++]; cmd->led.permanent.on_count = buf[pos++];
@ -210,11 +210,11 @@ static void pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len)
break; break;
} }
cmd->id = OSDP_CMD_BUZZER; cmd->id = OSDP_CMD_BUZZER;
cmd->buzzer.reader = buf[pos++]; cmd->buzzer.reader = buf[pos++];
cmd->buzzer.tone_code = buf[pos++]; cmd->buzzer.control_code = buf[pos++];
cmd->buzzer.on_count = buf[pos++]; cmd->buzzer.on_count = buf[pos++];
cmd->buzzer.off_count = buf[pos++]; cmd->buzzer.off_count = buf[pos++];
cmd->buzzer.rep_count = buf[pos++]; cmd->buzzer.rep_count = buf[pos++];
osdp_cmd_enqueue(pd, cmd); osdp_cmd_enqueue(pd, cmd);
pd->reply_id = REPLY_ACK; pd->reply_id = REPLY_ACK;
ret = 0; ret = 0;
@ -229,12 +229,12 @@ static void pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len)
break; break;
} }
cmd->id = OSDP_CMD_TEXT; cmd->id = OSDP_CMD_TEXT;
cmd->text.reader = buf[pos++]; cmd->text.reader = buf[pos++];
cmd->text.cmd = buf[pos++]; cmd->text.control_code = buf[pos++];
cmd->text.temp_time = buf[pos++]; cmd->text.temp_time = buf[pos++];
cmd->text.offset_row = buf[pos++]; cmd->text.offset_row = buf[pos++];
cmd->text.offset_col = buf[pos++]; cmd->text.offset_col = buf[pos++];
cmd->text.length = buf[pos++]; cmd->text.length = buf[pos++];
if (cmd->text.length > OSDP_CMD_TEXT_MAX_LEN || if (cmd->text.length > OSDP_CMD_TEXT_MAX_LEN ||
((len - CMD_TEXT_DATA_LEN) < cmd->text.length) || ((len - CMD_TEXT_DATA_LEN) < cmd->text.length) ||
cmd->text.length > OSDP_CMD_TEXT_MAX_LEN) { cmd->text.length > OSDP_CMD_TEXT_MAX_LEN) {
@ -258,16 +258,18 @@ static void pd_decode_command(struct osdp_pd *pd, uint8_t *buf, int len)
break; break;
} }
cmd->id = OSDP_CMD_COMSET; cmd->id = OSDP_CMD_COMSET;
cmd->comset.addr = buf[pos++]; cmd->comset.address = buf[pos++];
cmd->comset.baud = buf[pos++]; cmd->comset.baud_rate = buf[pos++];
cmd->comset.baud |= buf[pos++] << 8; cmd->comset.baud_rate |= buf[pos++] << 8;
cmd->comset.baud |= buf[pos++] << 16; cmd->comset.baud_rate |= buf[pos++] << 16;
cmd->comset.baud |= buf[pos++] << 24; cmd->comset.baud_rate |= buf[pos++] << 24;
if (cmd->comset.addr >= 0x7F || (cmd->comset.baud != 9600 && if (cmd->comset.address >= 0x7F ||
cmd->comset.baud != 38400 && cmd->comset.baud != 115200)) { (cmd->comset.baud_rate != 9600 &&
LOG_ERR("COMSET Failed! command discarded."); cmd->comset.baud_rate != 38400 &&
cmd->comset.addr = pd->address; cmd->comset.baud_rate != 115200)) {
cmd->comset.baud = pd->baud_rate; LOG_ERR(TAG "COMSET Failed! command discarded");
cmd->comset.address = pd->address;
cmd->comset.baud_rate = pd->baud_rate;
} }
osdp_cmd_enqueue(pd, cmd); osdp_cmd_enqueue(pd, cmd);
pd->reply_id = REPLY_COM; pd->reply_id = REPLY_COM;
@ -405,14 +407,14 @@ static int pd_build_reply(struct osdp_pd *pd, uint8_t *buf, int max_len)
} }
buf[len++] = pd->reply_id; buf[len++] = pd->reply_id;
buf[len++] = cmd->comset.addr; buf[len++] = cmd->comset.address;
buf[len++] = BYTE_0(cmd->comset.baud); buf[len++] = BYTE_0(cmd->comset.baud_rate);
buf[len++] = BYTE_1(cmd->comset.baud); buf[len++] = BYTE_1(cmd->comset.baud_rate);
buf[len++] = BYTE_2(cmd->comset.baud); buf[len++] = BYTE_2(cmd->comset.baud_rate);
buf[len++] = BYTE_3(cmd->comset.baud); buf[len++] = BYTE_3(cmd->comset.baud_rate);
pd->address = (int)cmd->comset.addr; pd->address = (int)cmd->comset.address;
pd->baud_rate = (int)cmd->comset.baud; pd->baud_rate = (int)cmd->comset.baud_rate;
LOG_INF("COMSET Succeeded! New PD-Addr: %d; Baud: %d", LOG_INF("COMSET Succeeded! New PD-Addr: %d; Baud: %d",
pd->address, pd->baud_rate); pd->address, pd->baud_rate);
ret = 0; ret = 0;