From 72a84f03631073bd9c165ee00efe54fe83155f7c Mon Sep 17 00:00:00 2001 From: David Fenyes Date: Wed, 11 Mar 2020 01:52:47 -0500 Subject: [PATCH] add safeguards for virtual device assignment Disallow assignment of invalid virtual or real devices Disallow assignment of a real devices that have already been assigned. add unit test cases for virtual devices --- firmware/asdf/src/asdf_virtual.c | 101 +++++++++++++++++--- firmware/asdf/test/test_asdf_virtual.c | 126 ++++++++++++++++++++++--- 2 files changed, 203 insertions(+), 24 deletions(-) diff --git a/firmware/asdf/src/asdf_virtual.c b/firmware/asdf/src/asdf_virtual.c index d5c2021..d45c78d 100644 --- a/firmware/asdf/src/asdf_virtual.c +++ b/firmware/asdf/src/asdf_virtual.c @@ -28,6 +28,7 @@ // #include +#include #include "asdf_virtual.h" #include "asdf_keymap_defs.h" #include "asdf_config.h" @@ -65,9 +66,9 @@ static void (*const vout_set[])(uint8_t) = { [VMAP_OUT1_OC] = &asdf_arch_out1_hi_z_set, // [VMAP_OUT2_OC] = &asdf_arch_out2_hi_z_set, // [VMAP_OUT3_OC] = &asdf_arch_out3_hi_z_set, // - [VMAP_LED1] = &asdf_arch_led1_set, // - [VMAP_LED2] = &asdf_arch_led2_set, // - [VMAP_LED3] = &asdf_arch_led3_set // + [VMAP_LED1] = &asdf_arch_led1_set, // + [VMAP_LED2] = &asdf_arch_led2_set, // + [VMAP_LED3] = &asdf_arch_led3_set // }; // virtual_out[] contains all the virtual outputs. An asdf_virtual_output_t @@ -213,6 +214,74 @@ void asdf_virtual_activate(asdf_virtual_output_t virtual_out) asdf_virtual_action(virtual_out, virtual_device_table[virtual_out].function); } +// PROCEDURE: valid_virtual_device +// INPUTS: (asdf_virtual_output_t) device +// OUTPUTS: returns true (1) if the device is valid, false (0) if not valid. +// +// DESCRIPTION: test to see if device is a valid device value. +// +// SIDE EFFECTS: +// +// NOTES: +// +// SCOPE: private +// +// COMPLEXITY: 1 +// +static uint8_t valid_virtual_device(asdf_virtual_output_t device) +{ + return (device > V_NULL && device < NUM_VIRTUAL_OUTPUTS); +} + +// PROCEDURE: valid_real_device +// INPUTS: (asdf_virtual_real_dev_t) device +// OUTPUTS: returns true (1) if the device is valid, false (0) if not valid. +// +// DESCRIPTION: test to see if device is a valid device value. +// +// SIDE EFFECTS: +// +// NOTES: +// +// SCOPE: private +// +// COMPLEXITY: 1 +// +static uint8_t valid_real_device(asdf_virtual_real_dev_t device) +{ + return (device > VMAP_NO_OUT && device < NUM_REAL_OUTPUTS); +} + +// PROCEDURE: real_device_is_available +// INPUTS: asdf_virtual_real_dev_t requiested_device +// OUTPUTS: returns VMAP_NO_OUT if device is alreay allocated. If not yet allocated, +// returns the index of the device in the available list before the requested +// device. +// +// DESCRIPTION: iterates through the linked list of available devices. If the +// requested_device is encountered, return the element before the requested +// device in the list. If the end of the list is reached, return VMAP_NO_OUT. +// +// SIDE EFFECTS: none +// +// NOTES: +// +// SCOPE: private +// +// COMPLEXITY: 3 +// +static uint8_t real_device_is_available(asdf_virtual_real_dev_t device) +{ + asdf_virtual_real_dev_t current_out = VMAP_NO_OUT; + asdf_virtual_real_dev_t next_out = real_device_table[current_out].next; + + while (next_out != VMAP_NO_OUT && next_out != device) { + current_out = next_out; + next_out = real_device_table[current_out].next; + } + return (VMAP_NO_OUT == next_out) ? NUM_REAL_OUTPUTS : current_out; +} + // PROCEDURE: asdf_virtual_assign // INPUTS: (asdf_vout_t) virtual_out // (uint8_t) real_out @@ -223,7 +292,8 @@ void asdf_virtual_activate(asdf_virtual_output_t virtual_out) // // SIDE EFFECTS: see above. // -// NOTES: +// NOTES: if the virtual device is invalid, or the real device is invalid, or +// the real device is already assigned, then nothing happens. // // SCOPE: private // @@ -232,16 +302,25 @@ void asdf_virtual_activate(asdf_virtual_output_t virtual_out) static void asdf_virtual_assign(asdf_virtual_output_t virtual_out, asdf_virtual_real_dev_t real_out, asdf_virtual_function_t function, uint8_t initial_value) { - virtual_device_table[virtual_out].function = function; + asdf_virtual_real_dev_t predecessor = real_device_is_available(real_out); - // add real device to the list associated with the virtual device: + if (valid_virtual_device(virtual_out) + && valid_real_device(real_out) + && (NUM_REAL_OUTPUTS != predecessor)) { + virtual_device_table[virtual_out].function = function; - real_device_table[real_out].next = virtual_device_table[virtual_out].real_device; - virtual_device_table[virtual_out].real_device = real_out; + // remove from available list: + real_device_table[predecessor].next = real_device_table[real_out].next; - // The real device shadow value is set here. The shadow values are asserted to - // the outputs only after all the assignments have been performed. - real_device_table[real_out].shadow = initial_value; + // add real device to the list associated with the virtual device: + + real_device_table[real_out].next = virtual_device_table[virtual_out].real_device; + virtual_device_table[virtual_out].real_device = real_out; + + // The real device shadow value is set here. The shadow values are asserted to + // the outputs only after all the assignments have been performed. + real_device_table[real_out].shadow = initial_value; + } } // PROCEDURE: asdf_virtual_init diff --git a/firmware/asdf/test/test_asdf_virtual.c b/firmware/asdf/test/test_asdf_virtual.c index 3a23dd9..ceee32a 100644 --- a/firmware/asdf/test/test_asdf_virtual.c +++ b/firmware/asdf/test/test_asdf_virtual.c @@ -46,7 +46,7 @@ void test_single_virtual_output_is_initialized(void) void test_uninitialized_virtual_out_is_default(void) { - TEST_ASSERT_EQUAL_INT32(ASDF_VIRTUAL_OUT_DEFAULT_VALUE, asdf_arch_check_output(VMAP_LED1)); + TEST_ASSERT_EQUAL_INT32(ASDF_VIRTUAL_OUT_DEFAULT_VALUE, asdf_arch_check_output(VMAP_LED2)); } void test_set_virtual_output(void) @@ -107,7 +107,8 @@ void test_pulse_low_virtual_output(void) // output. void test_toggle_triple_output(void) { - asdf_keymaps_select_keymap(1); + asdf_keymaps_select_keymap(TRIPLE_TESTS_KEYMAP); + // check that initial values have been set: TEST_ASSERT_EQUAL_INT32(0, asdf_arch_check_output(VMAP_OUT1)); TEST_ASSERT_EQUAL_INT32(1, asdf_arch_check_output(VMAP_OUT2)); @@ -128,7 +129,8 @@ void test_toggle_triple_output(void) // output high and low void test_set_triple_output(void) { - asdf_keymaps_select_keymap(1); + asdf_keymaps_select_keymap(TRIPLE_TESTS_KEYMAP); + // check that initial values have been set: TEST_ASSERT_EQUAL_INT32(0, asdf_arch_check_output(VMAP_OUT1)); TEST_ASSERT_EQUAL_INT32(1, asdf_arch_check_output(VMAP_OUT2)); @@ -149,7 +151,7 @@ void test_set_triple_output(void) // output high and low void test_pulse_triple_output(void) { - asdf_keymaps_select_keymap(1); + asdf_keymaps_select_keymap(TRIPLE_TESTS_KEYMAP); // check that initial values have been set: TEST_ASSERT_EQUAL_INT32(0, asdf_arch_check_output(VMAP_OUT1)); TEST_ASSERT_EQUAL_INT32(1, asdf_arch_check_output(VMAP_OUT2)); @@ -173,16 +175,111 @@ void test_pulse_triple_output(void) TEST_ASSERT_EQUAL_INT32(PD_ST_TRANSITION_LOW, asdf_arch_check_pulse(VMAP_OUT1)); TEST_ASSERT_EQUAL_INT32(PD_ST_TRANSITION_LOW, asdf_arch_check_pulse(VMAP_OUT2)); TEST_ASSERT_EQUAL_INT32(PD_ST_TRANSITION_LOW, asdf_arch_check_pulse(VMAP_OUT3)); - - // asdf_virtual_activate(VOUT1); // funtion is set to toggle - //TEST_ASSERT_EQUAL_INT32(1, asdf_arch_check_output(VMAP_OUT1)); - //TEST_ASSERT_EQUAL_INT32(0, asdf_arch_check_output(VMAP_OUT2)); - //TEST_ASSERT_EQUAL_INT32(1, asdf_arch_check_output(VMAP_OUT3)); +} - //asdf_virtual_action(VOUT1, V_TOGGLE); - //TEST_ASSERT_EQUAL_INT32(0, asdf_arch_check_output(VMAP_OUT1)); - //TEST_ASSERT_EQUAL_INT32(1, asdf_arch_check_output(VMAP_OUT2)); - //TEST_ASSERT_EQUAL_INT32(0, asdf_arch_check_output(VMAP_OUT3)); +uint8_t *output_array(void) +{ + static uint8_t outputs[NUM_REAL_OUTPUTS] = {}; + for (uint8_t i = 0; i < NUM_REAL_OUTPUTS; i++) { + outputs[i] = asdf_arch_check_output(i); + printf("output %d: %d\n", i, outputs[i]); + } + return outputs; +} + +uint8_t *all_set_array(void) +{ + static uint8_t outputs[NUM_REAL_OUTPUTS] = {}; + for (uint8_t i = 0; i < NUM_REAL_OUTPUTS; i++) { + outputs[i] = 1; + } + return outputs; +} + +uint8_t *all_zero_array(void) +{ + static uint8_t outputs[NUM_REAL_OUTPUTS] = {}; + for (uint8_t i = 0; i < NUM_REAL_OUTPUTS; i++) { + outputs[i] = 0; + } + return outputs; +} + +uint8_t *single_zero_array(asdf_virtual_real_dev_t set_element) +{ + static uint8_t outputs[NUM_REAL_OUTPUTS] = {}; + for (uint8_t i = 0; i < NUM_REAL_OUTPUTS; i++) { + outputs[i] = 1; + } + outputs[set_element] = 0; + return outputs; +} + + +void test_virtual_capslock_indicator(void) +{ + + asdf_keymaps_select_keymap(VCAPS_TEST_KEYMAP); + + // CAPS LED output should be initialized to zero: + TEST_ASSERT_EQUAL_INT32(0, asdf_arch_check_output(VMAP_LED1)); + + // emulate capslock press and release. Should set LED1 + asdf_modifier_capslock_activate(); + asdf_modifier_capslock_deactivate(); + + TEST_ASSERT_EQUAL_INT32(1, asdf_arch_check_output(VMAP_LED1)); + + + // emulate capslock press and release. clear LED1 + asdf_modifier_capslock_activate(); + asdf_modifier_capslock_deactivate(); + + TEST_ASSERT_EQUAL_INT32(0, asdf_arch_check_output(VMAP_LED1)); +} + +void test_virtual_shiftlock_indicator(void) +{ + + asdf_keymaps_select_keymap(VSHIFT_TEST_KEYMAP); + + // CAPS LED output should be initialized to zero: + TEST_ASSERT_EQUAL_INT32(0, asdf_arch_check_output(VMAP_LED2)); + + // emulate shiftlock press and release. Should set LED2 + asdf_modifier_shiftlock_activate(); + asdf_modifier_shiftlock_deactivate(); + + TEST_ASSERT_EQUAL_INT32(1, asdf_arch_check_output(VMAP_LED2)); + + + // emulate shift press and release. clear LED2 + asdf_modifier_shift_activate(); + asdf_modifier_shift_deactivate(); + + TEST_ASSERT_EQUAL_INT32(0, asdf_arch_check_output(VMAP_LED2)); +} + + +void test_cant_assign_real_output_twice(void) +{ + asdf_keymaps_select_keymap(DOUBLE_ASSIGN_TEST_KEYMAP); + + // initial value should be set to 0: + TEST_ASSERT_EQUAL_INT32(0, asdf_arch_check_output(VMAP_LED1)); + + // set LED1 high from valid VOUT4 + asdf_virtual_action(VOUT4, V_SET_HI); + TEST_ASSERT_EQUAL_INT32(1, asdf_arch_check_output(VMAP_LED1)); + + // set LED1 low from valid VOUT4 + asdf_virtual_action(VOUT4, V_SET_LO); + TEST_ASSERT_EQUAL_INT32(0, asdf_arch_check_output(VMAP_LED1)); + + // set LED1 high from invalid VOUT5 + asdf_virtual_action(VOUT5, V_SET_HI); + // Should not have changed. + TEST_ASSERT_EQUAL_INT32(0, asdf_arch_check_output(VMAP_LED1)); } int main(void) @@ -198,5 +295,8 @@ int main(void) RUN_TEST(test_toggle_triple_output); RUN_TEST(test_set_triple_output); RUN_TEST(test_pulse_triple_output); + RUN_TEST(test_virtual_capslock_indicator); + RUN_TEST(test_virtual_shiftlock_indicator); + RUN_TEST(test_cant_assign_real_output_twice); return UNITY_END(); }