diff --git a/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp b/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp index 9e084711..23c8067f 100644 --- a/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp +++ b/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp @@ -32,7 +32,8 @@ struct InternalData { extern "C" { static error_t read(Device* device, uint8_t address, uint8_t* data, size_t data_size, TickType_t timeout) { - vPortAssertIfInISR(); + if (xPortInIsrContext()) return ERROR_ISR_STATUS; + if (data_size == 0) return ERROR_INVALID_ARGUMENT; auto* driver_data = GET_DATA(device); lock(driver_data); const esp_err_t esp_error = i2c_master_read_from_device(GET_CONFIG(device)->port, address, data, data_size, timeout); @@ -41,18 +42,19 @@ static error_t read(Device* device, uint8_t address, uint8_t* data, size_t data_ return esp_err_to_error(esp_error); } -static error_t write(Device* device, uint8_t address, const uint8_t* data, uint16_t dataSize, TickType_t timeout) { - vPortAssertIfInISR(); +static error_t write(Device* device, uint8_t address, const uint8_t* data, uint16_t data_size, TickType_t timeout) { + if (xPortInIsrContext()) return ERROR_ISR_STATUS; + if (data_size == 0) return ERROR_INVALID_ARGUMENT; auto* driver_data = GET_DATA(device); lock(driver_data); - const esp_err_t esp_error = i2c_master_write_to_device(GET_CONFIG(device)->port, address, data, dataSize, timeout); + const esp_err_t esp_error = i2c_master_write_to_device(GET_CONFIG(device)->port, address, data, data_size, timeout); unlock(driver_data); ESP_ERROR_CHECK_WITHOUT_ABORT(esp_error); return esp_err_to_error(esp_error); } static error_t write_read(Device* device, uint8_t address, const uint8_t* write_data, size_t write_data_size, uint8_t* read_data, size_t read_data_size, TickType_t timeout) { - vPortAssertIfInISR(); + if (xPortInIsrContext()) return ERROR_ISR_STATUS; auto* driver_data = GET_DATA(device); lock(driver_data); const esp_err_t esp_error = i2c_master_write_read_device(GET_CONFIG(device)->port, address, write_data, write_data_size, read_data, read_data_size, timeout); @@ -61,7 +63,9 @@ static error_t write_read(Device* device, uint8_t address, const uint8_t* write_ return esp_err_to_error(esp_error); } -static error_t read_register(Device* device, uint8_t address, uint8_t reg, uint8_t* data, size_t dataSize, TickType_t timeout) { +static error_t read_register(Device* device, uint8_t address, uint8_t reg, uint8_t* data, size_t data_size, TickType_t timeout) { + if (xPortInIsrContext()) return ERROR_ISR_STATUS; + if (data_size == 0) return ERROR_INVALID_ARGUMENT; auto* driver_data = GET_DATA(device); lock(driver_data); @@ -73,10 +77,10 @@ static error_t read_register(Device* device, uint8_t address, uint8_t reg, uint8 // Read length of response from current pointer i2c_master_start(cmd); i2c_master_write_byte(cmd, (address << 1) | I2C_MASTER_READ, ACK_CHECK_EN); - if (dataSize > 1) { - i2c_master_read(cmd, data, dataSize - 1, I2C_MASTER_ACK); + if (data_size > 1) { + i2c_master_read(cmd, data, data_size - 1, I2C_MASTER_ACK); } - i2c_master_read_byte(cmd, data + dataSize - 1, I2C_MASTER_NACK); + i2c_master_read_byte(cmd, data + data_size - 1, I2C_MASTER_NACK); i2c_master_stop(cmd); // TODO: We're passing an inaccurate timeout value as we already lost time with locking esp_err_t esp_error = i2c_master_cmd_begin(GET_CONFIG(device)->port, cmd, timeout); @@ -87,7 +91,9 @@ static error_t read_register(Device* device, uint8_t address, uint8_t reg, uint8 return esp_err_to_error(esp_error); } -static error_t write_register(Device* device, uint8_t address, uint8_t reg, const uint8_t* data, uint16_t dataSize, TickType_t timeout) { +static error_t write_register(Device* device, uint8_t address, uint8_t reg, const uint8_t* data, uint16_t data_size, TickType_t timeout) { + if (xPortInIsrContext()) return ERROR_ISR_STATUS; + if (data_size == 0) return ERROR_INVALID_ARGUMENT; auto* driver_data = GET_DATA(device); lock(driver_data); @@ -95,7 +101,7 @@ static error_t write_register(Device* device, uint8_t address, uint8_t reg, cons i2c_master_start(cmd); i2c_master_write_byte(cmd, (address << 1) | I2C_MASTER_WRITE, ACK_CHECK_EN); i2c_master_write_byte(cmd, reg, ACK_CHECK_EN); - i2c_master_write(cmd, (uint8_t*) data, dataSize, ACK_CHECK_EN); + i2c_master_write(cmd, (uint8_t*) data, data_size, ACK_CHECK_EN); i2c_master_stop(cmd); // TODO: We're passing an inaccurate timeout value as we already lost time with locking esp_err_t esp_error = i2c_master_cmd_begin(GET_CONFIG(device)->port, cmd, timeout); diff --git a/TactilityKernel/Source/concurrent/thread.cpp b/TactilityKernel/Source/concurrent/thread.cpp index 35d78066..c7e6f5cb 100644 --- a/TactilityKernel/Source/concurrent/thread.cpp +++ b/TactilityKernel/Source/concurrent/thread.cpp @@ -43,10 +43,12 @@ struct Thread { static void thread_set_state_internal(Thread* thread, ThreadState newState) { thread->lock(); thread->state = newState; - if (thread->stateCallback) { - thread->stateCallback(thread->state, thread->stateCallbackContext); - } + auto cb = thread->stateCallback; + auto cb_ctx = thread->stateCallbackContext; thread->unlock(); + if (cb) { + cb(newState, cb_ctx); + } } static void thread_main_body(void* context) { @@ -57,18 +59,24 @@ static void thread_main_body(void* context) { check(pvTaskGetThreadLocalStoragePointer(nullptr, LOCAL_STORAGE_SELF_POINTER_INDEX) == nullptr); vTaskSetThreadLocalStoragePointer(nullptr, LOCAL_STORAGE_SELF_POINTER_INDEX, thread); - LOG_I(TAG, "Starting %s", thread->name.c_str()); + LOG_I(TAG, "Starting %s", thread->name.c_str()); // No need to lock as we don't allow mutation after thread start check(thread->state == THREAD_STATE_STARTING); thread_set_state_internal(thread, THREAD_STATE_RUNNING); - thread->callbackResult = thread->mainFunction(thread->mainFunctionContext); + int32_t result = thread->mainFunction(thread->mainFunctionContext); + thread->lock(); + thread->callbackResult = result; + thread->unlock(); check(thread->state == THREAD_STATE_RUNNING); thread_set_state_internal(thread, THREAD_STATE_STOPPED); - LOG_I(TAG, "Stopped %s", thread->name.c_str()); + LOG_I(TAG, "Stopped %s", thread->name.c_str()); // No need to lock as we don't allow mutation after thread start vTaskSetThreadLocalStoragePointer(nullptr, LOCAL_STORAGE_SELF_POINTER_INDEX, nullptr); + + thread->lock(); thread->taskHandle = nullptr; + thread->unlock(); vTaskDelete(nullptr); } @@ -117,7 +125,6 @@ void thread_set_name(Thread* thread, const char* name) { void thread_set_stack_size(Thread* thread, size_t stack_size) { thread->lock(); check(thread->state == THREAD_STATE_STOPPED); - check(stack_size % 4 == 0); thread->stackSize = stack_size; thread->unlock(); } @@ -208,7 +215,15 @@ error_t thread_start(Thread* thread) { ); } - return (result == pdPASS) ? ERROR_NONE : ERROR_UNDEFINED; + if (result != pdPASS) { + thread_set_state_internal(thread, THREAD_STATE_STOPPED); + thread->lock(); + thread->taskHandle = nullptr; + thread->unlock(); + return ERROR_UNDEFINED; + } + + return ERROR_NONE; } error_t thread_join(Thread* thread, TickType_t timeout, TickType_t poll_interval) { diff --git a/TactilityKernel/Source/drivers/i2c_controller.cpp b/TactilityKernel/Source/drivers/i2c_controller.cpp index 50012df0..be3af8c7 100644 --- a/TactilityKernel/Source/drivers/i2c_controller.cpp +++ b/TactilityKernel/Source/drivers/i2c_controller.cpp @@ -35,7 +35,9 @@ error_t i2c_controller_write_register(Device* device, uint8_t address, uint8_t r error_t i2c_controller_write_register_array(Device* device, uint8_t address, const uint8_t* data, uint16_t dataSize, TickType_t timeout) { const auto* driver = device_get_driver(device); - assert(dataSize % 2 == 0); + if (dataSize % 2 != 0) { + return ERROR_INVALID_ARGUMENT; + } for (int i = 0; i < dataSize; i += 2) { error_t error = I2C_DRIVER_API(driver)->write_register(device, address, data[i], &data[i + 1], 1, timeout); if (error != ERROR_NONE) return error;