common-io: Refactoring and updates to Temp Sensor test cases #1932
Conversation
| @@ -79,6 +81,16 @@ TEST_TEAR_DOWN( TEST_IOT_TSENSOR ) | |||
| */ | |||
| TEST_GROUP_RUNNER( TEST_IOT_TSENSOR ) | |||
| { | |||
| if( xtestIotTsensorMinSemaphore == NULL ) | |||
| { | |||
| xtestIotTsensorMinSemaphore = xSemaphoreCreateBinary(); | |||
There was a problem hiding this comment.
In general, you should always check return codes here since xSemaphoreCreateBinary() can return NULL.
There was a problem hiding this comment.
It makes sense. I moved the block out to the test setup phase and added asserts to check against NULL return.
|
/bot run checks |
…s and reworked AFQP_IotTsensorTriggerMinThreshold and AFQP_IotTsensorTriggerMaxThreshold. Updated expected return values based on what is defined by the API header file
…heck for NULL return
qiutongs
force-pushed
the
fix/common-io-tsensor-test
branch
from
a55e130 to
acaae4f
Compare
|
/bot run checks |
1 similar comment
|
/bot run checks |
| @@ -61,6 +63,17 @@ TEST_GROUP( TEST_IOT_TSENSOR ); | |||
| */ | |||
| TEST_SETUP( TEST_IOT_TSENSOR ) | |||
| { | |||
| if( xtestIotTsensorMinSemaphore == NULL ) | |||
There was a problem hiding this comment.
It think the test should check this and assert and fail if it already exists. What will happen if a prior test hands you a locked semaphore?
There was a problem hiding this comment.
There are a couple of these further down also, this applies to all of them.
|
/bot run checks |
| @@ -39,6 +39,7 @@ | |||
| #include "FreeRTOS.h" | |||
| #include "semphr.h" | |||
|
|
|||
| #define testIotTSENSOR_DELAY pdMS_TO_TICKS( 5000 ) | |||
There was a problem hiding this comment.
In general when a #define macro or variable has units, add the units to the end of the name. This helps reviewers and maintainers spot misuse. Here we would prefer the name testIoTSENSOR_DELAY_TICKS.
| lRetVal = iot_tsensor_ioctl( xTsensorHandle, eTsensorSetMinThreshold, &setLowThreshold ); | ||
| /* Set min threshold to be 5 degrees C higher than current temperature. | ||
| * Expect it to fire immediately */ | ||
| setLowThreshold += 5000; |
There was a problem hiding this comment.
These values are in millidegrees Celsius, consider a naming convention so that maintainers/readers can ensure the values are used & converted properly.
* Refactored and updated tsensor test cases. Added TEST_PROTECT sections and reworked AFQP_IotTsensorTriggerMinThreshold and AFQP_IotTsensorTriggerMaxThreshold. Updated expected return values based on what is defined by the API header file * Updated AFQP_IotTsensorCalibrationSuccess to test NULL buffer * Moved semaphore creation into setup phase and added test asserts to check for NULL return * Create/delete semaphore for each test case Co-authored-by: Max Wennerfeldt <[email protected]> Co-authored-by: qiutongs <songqt01@gmail,com>
Description
Checklist:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.