diff --git a/.restyled.yaml b/.restyled.yaml index c45fe98588b7ec..d4ca39f3efa448 100644 --- a/.restyled.yaml +++ b/.restyled.yaml @@ -98,6 +98,7 @@ restylers: - "**/*.c" - "**/*.cc" - "**/*.cpp" + - "**/*.ipp" - "**/*.cxx" - "**/*.c++" - "**/*.C" @@ -137,6 +138,7 @@ restylers: - "**/*.c" - "**/*.cc" - "**/*.cpp" + - "**/*.ipp" - "**/*.cxx" - "**/*.c++" - "**/*.C" diff --git a/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h b/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h index 617e5caa39fa33..f1a400fc884efd 100644 --- a/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h +++ b/src/include/platform/internal/GenericPlatformManagerImpl_FreeRTOS.h @@ -60,9 +60,9 @@ class GenericPlatformManagerImpl_FreeRTOS : public GenericPlatformManagerImpl::_InitChipStack(void) vTaskSetTimeOutState(&mNextTimerBaseTime); mNextTimerDurationTicks = 0; - mEventLoopTask = NULL; - mChipTimerActive = false; + // TODO: This nulling out of mEventLoopTask should happen when we shut down + // the task, not here! + mEventLoopTask = NULL; + mChipTimerActive = false; + // We support calling Shutdown followed by InitChipStack, because some tests + // do that. To keep things simple for existing consumers, we keep not + // destroying our lock and queue in shutdown, but rather check whether they + // already exist here before trying to create them. + + if (mChipStackLock == NULL) + { #if defined(CHIP_CONFIG_FREERTOS_USE_STATIC_SEMAPHORE) && CHIP_CONFIG_FREERTOS_USE_STATIC_SEMAPHORE - mChipStackLock = xSemaphoreCreateMutexStatic(&mChipStackLockMutex); + mChipStackLock = xSemaphoreCreateMutexStatic(&mChipStackLockMutex); #else - mChipStackLock = xSemaphoreCreateMutex(); + mChipStackLock = xSemaphoreCreateMutex(); #endif // CHIP_CONFIG_FREERTOS_USE_STATIC_SEMAPHORE - if (mChipStackLock == NULL) - { - ChipLogError(DeviceLayer, "Failed to create CHIP stack lock"); - ExitNow(err = CHIP_ERROR_NO_MEMORY); + if (mChipStackLock == NULL) + { + ChipLogError(DeviceLayer, "Failed to create CHIP stack lock"); + ExitNow(err = CHIP_ERROR_NO_MEMORY); + } } + if (mChipEventQueue == NULL) + { #if defined(CHIP_CONFIG_FREERTOS_USE_STATIC_QUEUE) && CHIP_CONFIG_FREERTOS_USE_STATIC_QUEUE - mChipEventQueue = - xQueueCreateStatic(CHIP_DEVICE_CONFIG_MAX_EVENT_QUEUE_SIZE, sizeof(ChipDeviceEvent), mEventQueueBuffer, &mEventQueueStruct); + mChipEventQueue = xQueueCreateStatic(CHIP_DEVICE_CONFIG_MAX_EVENT_QUEUE_SIZE, sizeof(ChipDeviceEvent), mEventQueueBuffer, + &mEventQueueStruct); #else - mChipEventQueue = xQueueCreate(CHIP_DEVICE_CONFIG_MAX_EVENT_QUEUE_SIZE, sizeof(ChipDeviceEvent)); + mChipEventQueue = xQueueCreate(CHIP_DEVICE_CONFIG_MAX_EVENT_QUEUE_SIZE, sizeof(ChipDeviceEvent)); #endif - if (mChipEventQueue == NULL) + if (mChipEventQueue == NULL) + { + ChipLogError(DeviceLayer, "Failed to allocate CHIP event queue"); + ExitNow(err = CHIP_ERROR_NO_MEMORY); + } + } + else { - ChipLogError(DeviceLayer, "Failed to allocate CHIP event queue"); - ExitNow(err = CHIP_ERROR_NO_MEMORY); + // Clear out any events that might be stuck in the queue, so we start + // with a clean slate, as if we had just re-created the queue. + xQueueReset(mChipEventQueue); } mShouldRunEventLoop.store(false); @@ -145,7 +164,7 @@ void GenericPlatformManagerImpl_FreeRTOS::_RunEventLoop(void) ChipDeviceEvent event; // Lock the CHIP stack. - Impl()->LockChipStack(); + StackLock lock; bool oldShouldRunEventLoop = false; if (!mShouldRunEventLoop.compare_exchange_strong(oldShouldRunEventLoop /* expected */, true /* desired */)) @@ -198,13 +217,13 @@ void GenericPlatformManagerImpl_FreeRTOS::_RunEventLoop(void) waitTime = portMAX_DELAY; } - // Unlock the CHIP stack, allowing other threads to enter CHIP while the event loop thread is sleeping. - Impl()->UnlockChipStack(); - - BaseType_t eventReceived = xQueueReceive(mChipEventQueue, &event, waitTime); - - // Lock the CHIP stack. - Impl()->LockChipStack(); + BaseType_t eventReceived = pdFALSE; + { + // Unlock the CHIP stack, allowing other threads to enter CHIP while + // the event loop thread is sleeping. + StackUnlock unlock; + eventReceived = xQueueReceive(mChipEventQueue, &event, waitTime); + } // If an event was received, dispatch it. Continue receiving events from the queue and // dispatching them until the queue is empty. @@ -236,6 +255,9 @@ void GenericPlatformManagerImpl_FreeRTOS::EventLoopTaskMain(void * ar { ChipLogDetail(DeviceLayer, "CHIP task running"); static_cast *>(arg)->Impl()->RunEventLoop(); + // TODO: At this point, should we not + // vTaskDelete(static_cast *>(arg)->mEventLoopTask)? + // Or somehow get our caller to do it once this thread is joined? } template @@ -275,6 +297,7 @@ void GenericPlatformManagerImpl_FreeRTOS::PostEventFromISR(const Chip template void GenericPlatformManagerImpl_FreeRTOS::_Shutdown(void) { + GenericPlatformManagerImpl::_Shutdown(); } template diff --git a/src/platform/ESP32/PlatformManagerImpl.cpp b/src/platform/ESP32/PlatformManagerImpl.cpp index d2254692924cca..10f9554477bf13 100644 --- a/src/platform/ESP32/PlatformManagerImpl.cpp +++ b/src/platform/ESP32/PlatformManagerImpl.cpp @@ -150,6 +150,8 @@ void PlatformManagerImpl::_Shutdown() } Internal::GenericPlatformManagerImpl_FreeRTOS::_Shutdown(); + + esp_event_loop_delete_default(); } void PlatformManagerImpl::HandleESPSystemEvent(void * arg, esp_event_base_t eventBase, int32_t eventId, void * eventData) diff --git a/src/system/tests/BUILD.gn b/src/system/tests/BUILD.gn index 51f321ef8dfe94..9d4e1c2b7958f3 100644 --- a/src/system/tests/BUILD.gn +++ b/src/system/tests/BUILD.gn @@ -31,7 +31,7 @@ chip_test_suite("tests") { "TestTimeSource.cpp", ] - if (chip_device_platform != "esp32" && chip_device_platform != "fake") { + if (chip_device_platform != "fake") { test_sources += [ "TestSystemScheduleWork.cpp" ] } diff --git a/src/test_driver/esp32/run_qemu_image.py b/src/test_driver/esp32/run_qemu_image.py index d871083a0c1c77..698649fc64c5d7 100755 --- a/src/test_driver/esp32/run_qemu_image.py +++ b/src/test_driver/esp32/run_qemu_image.py @@ -17,6 +17,7 @@ import click import logging import os +import re import sys import subprocess @@ -104,19 +105,41 @@ def main(log_level, no_log_timestamps, image, file_image_list, qemu, verbose): (path, status.returncode)) # Parse output of the unit test. Generally expect things like: + # I (3034) CHIP-tests: Starting CHIP tests! + # ... + # Various test lines, none ending with "] : FAILED" + # ... # Failed Tests: 0 / 5 # Failed Asserts: 0 / 77 # I (3034) CHIP-tests: CHIP test status: 0 + in_test = False for line in output.split('\n'): + if line.endswith('CHIP-tests: Starting CHIP tests!'): + in_test = True + if line.startswith('Failed Tests:') and not line.startswith('Failed Tests: 0 /'): raise Exception("Tests seem failed: %s" % line) if line.startswith('Failed Asserts:') and not line.startswith('Failed Asserts: 0 /'): raise Exception("Asserts seem failed: %s" % line) - if 'CHIP test status: ' in line and 'CHIP test status: 0' not in line: + if 'CHIP-tests: CHIP test status: 0' in line: + in_test = False + elif 'CHIP-tests: CHIP test status: ' in line: raise Exception("CHIP test status is NOT 0: %s" % line) + # Ignore FAILED messages not in the middle of a test, to reduce + # the chance of false posisitves from other logging. + if in_test and re.match('.*] : FAILED$', line): + raise Exception("Step failed: %s" % line) + + # TODO: Figure out why exit(0) in man_app.cpp's tester_task is aborting and fix that. + if in_test and line.startswith('abort() was called at PC'): + raise Exception("Unexpected crash: %s" % line) + + if in_test: + raise Exception('Not expected to be in the middle of a test when the log ends') + if verbose: print("========== TEST OUTPUT BEGIN ============") print(output) @@ -125,7 +148,9 @@ def main(log_level, no_log_timestamps, image, file_image_list, qemu, verbose): logging.info("Image %s PASSED", path) except: # make sure output is visible in stdout + print("========== TEST OUTPUT BEGIN ============") print(output) + print("========== TEST OUTPUT END ============") raise