diff --git a/gt.h b/gt.h new file mode 100644 index 0000000..480cac5 --- /dev/null +++ b/gt.h @@ -0,0 +1,302 @@ +aZholtikov: что можно добавить или улучшить? + +GitHub Copilot: Ваш код выглядит достаточно хорошо структурированным и функциональным, но есть несколько улучшений и дополнений, которые можно рассмотреть: + +### 1. **Проверка ошибок и логирование** + - Добавьте более детализированные сообщения об ошибках, чтобы упростить отладку. Например, укажите, какой именно параметр вызвал ошибку. + - Используйте макросы для логирования, чтобы избежать дублирования кода. + +### 2. **Улучшение читаемости** + - Разделите длинные строки кода, такие как `handle->gpio_work_mode` в функции `zh_pcf8574_init`, на несколько строк для улучшения читаемости. + - Добавьте комментарии к сложным участкам кода, чтобы объяснить их назначение. + +### 3. **Обработка ошибок** + - В некоторых местах, например, при создании задачи `_zh_isr_processing_task`, вы возвращаете `ESP_FAIL`, но не освобождаете ресурсы, такие как семафоры. Добавьте очистку ресурсов в случае ошибок. + +### 4. **Тестирование** + - Добавьте модульные тесты для функций, таких как `zh_pcf8574_read`, `zh_pcf8574_write`, и других. Это поможет убедиться, что код работает корректно в различных сценариях. + +### 5. **Оптимизация ISR** + - В обработчике прерываний `_zh_isr_handler` можно добавить проверку на `NULL` для `_interrupt_semaphore`, чтобы избежать потенциальных ошибок. + +### 6. **Конфигурация I2C** + - Позвольте пользователю задавать скорость I2C (например, через конфигурацию), вместо жестко заданного значения `100000`. + +### 7. **Улучшение API** + - Добавьте функцию для изменения конфигурации GPIO после инициализации, чтобы пользователи могли динамически изменять режим работы пинов. + +### 8. **Документация** + - Расширьте комментарии в заголовочном файле, добавив примеры использования функций. + - Укажите ограничения, такие как максимальное количество поддерживаемых устройств. + +### 9. **Проверка входных данных** + - В функциях, таких как `zh_pcf8574_write_gpio`, добавьте проверку на корректность входных данных (например, проверку, что `gpio` находится в допустимом диапазоне). + +### 10. **Добавление функции деинициализации** + - Добавьте функцию `zh_pcf8574_deinit`, которая будет освобождать ресурсы, такие как семафоры, задачи и обработчики прерываний. + +Пример функции деинициализации: +```c +esp_err_t zh_pcf8574_deinit(zh_pcf8574_handle_t *handle) +{ + if (handle == NULL || !handle->is_initialized) + { + ESP_LOGE(TAG, "PCF8574 deinit fail. Invalid argument or not initialized."); + return ESP_ERR_INVALID_ARG; + } + + if (_interrupt_gpio != GPIO_NUM_MAX) + { + gpio_isr_handler_remove(_interrupt_gpio); + vSemaphoreDelete(_interrupt_semaphore); + _interrupt_gpio = GPIO_NUM_MAX; + } + + handle->is_initialized = false; + ESP_LOGI(TAG, "PCF8574 deinitialized successfully."); + return ESP_OK; +} +``` + +### 11. **Поддержка нескольких устройств** + - Если планируется использование нескольких PCF8574, добавьте механизм для управления несколькими устройствами, например, через массив дескрипторов. + +### 12. **Улучшение производительности** + - В функции `_zh_isr_processing_task` можно оптимизировать цикл, чтобы не проверять все GPIO, если известно, какой из них вызвал прерывание. + +### 13. **Кроссплатформенность** + - Добавьте макросы для упрощения поддержки разных платформ (например, ESP32 и ESP8266). + +### 14. **Использование статического анализа** + - Проверьте код с помощью инструментов статического анализа, таких как `cppcheck` или встроенные инструменты ESP-IDF, чтобы выявить потенциальные проблемы. + +Если хотите, я могу помочь реализовать одно из предложений. + +aZholtikov: _zh_isr_processing_task оптимизировать цикл, чтобы не проверять все GPIO, если известно, какой из них вызвал прерывание. + +GitHub Copilot: Для оптимизации цикла в функции `_zh_isr_processing_task` можно использовать информацию о том, какой GPIO вызвал прерывание, чтобы избежать проверки всех GPIO. Это можно сделать, если в обработчике прерываний `_zh_isr_handler` передавать номер GPIO, вызвавшего прерывание, через аргумент или глобальную переменную. Вот пример оптимизации: + +### Изменения в коде: + +1. **Добавить глобальную переменную для хранения номера GPIO, вызвавшего прерывание:** + ```c + static uint8_t _triggered_gpio = 0xFF; // GPIO, вызвавший прерывание + ``` + +2. **Модифицировать обработчик прерываний `_zh_isr_handler`:** + В обработчике прерываний сохраняем номер GPIO, вызвавшего прерывание: + ```c + void IRAM_ATTR _zh_isr_handler(void *arg) + { + _triggered_gpio = _interrupt_gpio; // Сохраняем номер GPIO + BaseType_t xHigherPriorityTaskWoken = pdFALSE; + xSemaphoreGiveFromISR(_interrupt_semaphore, &xHigherPriorityTaskWoken); + if (xHigherPriorityTaskWoken == pdTRUE) + { + portYIELD_FROM_ISR(); + } + } + ``` + +3. **Оптимизировать цикл в `_zh_isr_processing_task`:** + Используем `_triggered_gpio` для проверки только соответствующего GPIO: + ```c + void _zh_isr_processing_task(void *pvParameter) + { + for (;;) + { + xSemaphoreTake(_interrupt_semaphore, portMAX_DELAY); + ESP_LOGI(TAG, "PCF8574 isr processing begin."); + + for (uint16_t i = 0; i < zh_vector_get_size(&_vector); ++i) + { + zh_pcf8574_handle_t *handle = zh_vector_get_item(&_vector, i); + zh_pcf8574_event_on_isr_t event = {0}; + event.i2c_address = handle->i2c_address; + event.gpio_number = 0xFF; + + uint8_t reg_temp = 0; + _zh_read_register(handle, ®_temp); + + // Проверяем только GPIO, вызвавший прерывание + if (((handle->gpio_work_mode & (1 << _triggered_gpio)) != 0) && + ((reg_temp & (1 << _triggered_gpio)) == 0)) + { + event.gpio_number = _triggered_gpio; + } + + if (event.gpio_number != 0xFF) + { + if (esp_event_post(ZH_PCF8574, 0, &event, sizeof(event), portTICK_PERIOD_MS) != ESP_OK) + { + ESP_LOGE(TAG, "PCF8574 isr processing task internal error at line %d.", __LINE__); + } + } + } + + ESP_LOGI(TAG, "PCF8574 isr processing success."); + } + vTaskDelete(NULL); + } + ``` + +### Объяснение изменений: +- **Глобальная переменная `_triggered_gpio`:** Хранит номер GPIO, вызвавшего прерывание, чтобы использовать его в задаче обработки. +- **Обработчик `_zh_isr_handler`:** Сохраняет номер GPIO, вызвавшего прерывание, в `_triggered_gpio`. +- **Оптимизация цикла:** Вместо проверки всех GPIO проверяется только тот, который вызвал прерывание, что значительно сокращает количество операций. + +### Преимущества: +- Уменьшается количество проверок в задаче обработки. +- Улучшается производительность, особенно при большом количестве устройств или GPIO. + +aZholtikov: добавить макросы обработки ошибок + +GitHub Copilot: Для упрощения обработки ошибок и улучшения читаемости кода можно добавить макросы для логирования и возврата ошибок. Вот пример: + +### Добавление макросов в `zh_pcf8574.h`: +```c +#pragma once + +#include "esp_log.h" +#include "esp_err.h" + +// Макрос для проверки аргументов +#define ZH_CHECK_ARG(ARG) \ + do { \ + if (!(ARG)) { \ + ESP_LOGE(TAG, "%s(%d): Invalid argument", \ + __FUNCTION__, __LINE__); \ + return ESP_ERR_INVALID_ARG; \ + } \ + } while (0) + +// Макрос для проверки результата вызова функции +#define ZH_CHECK_RESULT(CALL) \ + do { \ + esp_err_t __err_rc = (CALL); \ + if (__err_rc != ESP_OK) { \ + ESP_LOGE(TAG, "%s(%d): Error 0x%x", \ + __FUNCTION__, __LINE__, __err_rc); \ + return __err_rc; \ + } \ + } while (0) + +// Макрос для логирования и возврата ошибки +#define ZH_RETURN_ERROR(ERR, MSG) \ + do { \ + ESP_LOGE(TAG, "%s(%d): %s", __FUNCTION__, __LINE__, MSG); \ + return ERR; \ + } while (0) +``` + +### Пример использования макросов в zh_pcf8574.c: + +#### Функция `zh_pcf8574_init`: +```c +esp_err_t zh_pcf8574_init(const zh_pcf8574_init_config_t *config, zh_pcf8574_handle_t *handle) +{ + ESP_LOGI(TAG, "PCF8574 initialization begin."); + ZH_CHECK_ARG(config != NULL); + ZH_CHECK_ARG(handle != NULL); + ZH_CHECK_ARG(config->i2c_address != 0xFF); + + handle->i2c_address = config->i2c_address; + handle->gpio_work_mode = (config->p7_gpio_work_mode << 7) | (config->p6_gpio_work_mode << 6) | + (config->p5_gpio_work_mode << 5) | (config->p4_gpio_work_mode << 4) | + (config->p3_gpio_work_mode << 3) | (config->p2_gpio_work_mode << 2) | + (config->p1_gpio_work_mode << 1) | (config->p0_gpio_work_mode << 0); + handle->gpio_status = handle->gpio_work_mode; + handle->i2c_port = config->i2c_port; + +#ifndef CONFIG_IDF_TARGET_ESP8266 + handle->i2c_handle = config->i2c_handle; + i2c_device_config_t pcf8574_config = { + .dev_addr_length = I2C_ADDR_BIT_LEN_7, + .device_address = handle->i2c_address, + .scl_speed_hz = 100000, + }; + ZH_CHECK_RESULT(i2c_master_bus_add_device(handle->i2c_handle, &pcf8574_config, &handle->dev_handle)); + if (i2c_master_probe(handle->i2c_handle, handle->i2c_address, 1000 / portTICK_PERIOD_MS) != ESP_OK) + { + ZH_RETURN_ERROR(ESP_ERR_NOT_FOUND, "Expander not connected or not responded."); + } +#endif + + handle->is_initialized = true; + ZH_CHECK_RESULT(_zh_write_register(handle, handle->gpio_work_mode)); + + if (_interrupt_gpio == GPIO_NUM_MAX && config->interrupt_gpio != GPIO_NUM_MAX) + { + zh_vector_init(&_vector, sizeof(zh_pcf8574_handle_t)); + _interrupt_gpio = config->interrupt_gpio; + + gpio_config_t isr_pin_config = { + .intr_type = GPIO_INTR_NEGEDGE, + .mode = GPIO_MODE_INPUT, + .pin_bit_mask = (1ULL << _interrupt_gpio), + .pull_down_en = GPIO_PULLDOWN_DISABLE, + .pull_up_en = GPIO_PULLUP_ENABLE, + }; + if (gpio_config(&isr_pin_config) != ESP_OK) + { + ESP_LOGW(TAG, "GPIO driver error at line %d.", __LINE__); + } + else + { + _interrupt_semaphore = xSemaphoreCreateBinary(); + if (xTaskCreatePinnedToCore(&_zh_isr_processing_task, "_zh_isr_processing_task", config->stack_size, NULL, config->task_priority, NULL, tskNO_AFFINITY) != pdPASS) + { + ZH_RETURN_ERROR(ESP_FAIL, "Failed to create ISR processing task."); + } + gpio_install_isr_service(0); + gpio_isr_handler_add(_interrupt_gpio, _zh_isr_handler, NULL); + } + } + + if (_interrupt_gpio != GPIO_NUM_MAX && handle->gpio_work_mode != 0) + { + zh_vector_push_back(&_vector, handle); + } + + ESP_LOGI(TAG, "PCF8574 initialization success."); + return ESP_OK; +} +``` + +#### Функция `_zh_read_register`: +```c +esp_err_t _zh_read_register(zh_pcf8574_handle_t *handle, uint8_t *reg) +{ + ESP_LOGI(TAG, "PCF8574 read begin."); + ZH_CHECK_ARG(handle != NULL); + ZH_CHECK_ARG(reg != NULL); + + if (!handle->is_initialized) + { + ZH_RETURN_ERROR(ESP_ERR_NOT_FOUND, "PCF8574 not initialized."); + } + + esp_err_t err = ESP_OK; +#ifdef CONFIG_IDF_TARGET_ESP8266 + i2c_cmd_handle_t i2c_cmd_handle = i2c_cmd_link_create(); + i2c_master_start(i2c_cmd_handle); + i2c_master_write_byte(i2c_cmd_handle, handle->i2c_address << 1 | I2C_MASTER_READ, true); + i2c_master_read_byte(i2c_cmd_handle, &handle->gpio_status, I2C_MASTER_NACK); + i2c_master_stop(i2c_cmd_handle); + err = i2c_master_cmd_begin(handle->i2c_port, i2c_cmd_handle, 1000 / portTICK_PERIOD_MS); + i2c_cmd_link_delete(i2c_cmd_handle); +#else + err = i2c_master_receive(handle->dev_handle, &handle->gpio_status, sizeof(handle->gpio_status), 1000 / portTICK_PERIOD_MS); +#endif + ZH_CHECK_RESULT(err); + + *reg = handle->gpio_status; + ESP_LOGI(TAG, "PCF8574 read success."); + return ESP_OK; +} +``` + +### Преимущества: +1. **Улучшение читаемости:** Макросы сокращают повторяющийся код и делают функции более компактными. +2. **Единообразие:** Все ошибки обрабатываются одинаково, что упрощает отладку. +3. **Меньше дублирования:** Логика проверки аргументов и результатов вызовов вынесена в макросы. diff --git a/zh_pcf8574.c b/zh_pcf8574.c index 907948f..ad39644 100644 --- a/zh_pcf8574.c +++ b/zh_pcf8574.c @@ -2,11 +2,29 @@ static const char *TAG = "zh_pcf8574"; +#define ZH_PCF8574_LOGI(msg, ...) ESP_LOGI(TAG, msg, ##__VA_ARGS__) +#define ZH_PCF8574_LOGW(msg, ...) ESP_LOGW(TAG, msg, ##__VA_ARGS__) +#define ZH_PCF8574_LOGE(msg, ...) ESP_LOGE(TAG, msg, ##__VA_ARGS__) +#define ZH_PCF8574_LOGE_ERR(msg, err, ...) ESP_LOGE(TAG, "[%s:%d:%s] " msg, __FILE__, __LINE__, esp_err_to_name(err), ##__VA_ARGS__) + +#define ZH_PCF8574_CHECK(cond, err, msg, ...) \ + if (!(cond)) \ + { \ + ZH_PCF8574_LOGE_ERR(msg, err); \ + return err; \ + } + static gpio_num_t _interrupt_gpio = GPIO_NUM_MAX; static SemaphoreHandle_t _interrupt_semaphore = {0}; static zh_vector_t _vector = {0}; +// static esp_err_t _zh_espnow_init_wifi(const zh_espnow_init_config_t *config); +// static esp_err_t _zh_espnow_init_resources(const zh_espnow_init_config_t *config); +static esp_err_t _zh_pcf857_validate_config(const zh_pcf8574_init_config_t *config); +// static esp_err_t _zh_espnow_register_callbacks(bool battery_mode); +// static esp_err_t _zh_espnow_create_task(const zh_espnow_init_config_t *config); + static void _zh_isr_handler(void *arg); static void _zh_isr_processing_task(void *pvParameter); static esp_err_t _zh_read_register(zh_pcf8574_handle_t *handle, uint8_t *reg); @@ -16,15 +34,28 @@ ESP_EVENT_DEFINE_BASE(ZH_PCF8574); esp_err_t zh_pcf8574_init(const zh_pcf8574_init_config_t *config, zh_pcf8574_handle_t *handle) { - ESP_LOGI(TAG, "PCF8574 initialization begin."); - if (config == NULL || handle == NULL || config->i2c_address == 0xFF) + ZH_PCF8574_LOGI("PCF8574 initialization started."); + if (handle != NULL && handle->is_initialized == true) { - ESP_LOGE(TAG, "PCF8574 initialization fail. Invalid argument."); - return ESP_ERR_INVALID_ARG; + ZH_PCF8574_LOGW("PCF8574 initialization failed. PCF8574 is already initialized."); + return ESP_OK; } + esp_err_t err = _zh_pcf8574_validate_config(config); + if (err != ESP_OK) + { + ZH_PCF8574_LOGE("PCF8574 initialization failed. Initial configuration check failed."); + return err; + } + else + { + ZH_PCF8574_LOGI("PCF8574 initial configuration check completed successfully."); + } + + ZH_PCF8574_CHECK((config != NULL || handle != NULL || config->i2c_address != 0xFF), ESP_ERR_INVALID_ARG, "PCF8574 initialization fail. Invalid argument."); handle->i2c_address = config->i2c_address; - handle->gpio_work_mode = (config->p7_gpio_work_mode << 7) | (config->p6_gpio_work_mode << 6) | (config->p5_gpio_work_mode << 5) | (config->p4_gpio_work_mode << 4) | - (config->p3_gpio_work_mode << 3) | (config->p2_gpio_work_mode << 2) | (config->p1_gpio_work_mode << 1) | (config->p0_gpio_work_mode << 0); + handle->gpio_work_mode = (config->p7_gpio_work_mode << 7) | (config->p6_gpio_work_mode << 6) | (config->p5_gpio_work_mode << 5) | + (config->p4_gpio_work_mode << 4) | (config->p3_gpio_work_mode << 3) | (config->p2_gpio_work_mode << 2) | + (config->p1_gpio_work_mode << 1) | (config->p0_gpio_work_mode << 0); handle->gpio_status = handle->gpio_work_mode; handle->i2c_port = config->i2c_port; #ifndef CONFIG_IDF_TARGET_ESP8266 @@ -117,6 +148,16 @@ esp_err_t zh_pcf8574_write_gpio(zh_pcf8574_handle_t *handle, zh_pcf8574_gpio_num } } +static esp_err_t _zh_pcf857_validate_config(const zh_pcf8574_init_config_t *config) +{ + ZH_PCF8574_CHECK(config != NULL, ESP_ERR_INVALID_ARG, "Invalid configuration."); + // ZH_ESPNOW_CHECK(config->wifi_channel > 0 && config->wifi_channel < 15, ESP_ERR_INVALID_ARG, "Invalid WiFi channel."); + // ZH_ESPNOW_CHECK(config->task_priority >= 5 && config->stack_size >= 2048, ESP_ERR_INVALID_ARG, "Invalid task settings."); + // ZH_ESPNOW_CHECK(config->queue_size >= 16, ESP_ERR_INVALID_ARG, "Invalid queue size."); + // ZH_ESPNOW_CHECK(config->attempts > 0, ESP_ERR_INVALID_ARG, "Invalid number of attempts."); + return ESP_OK; +} + void IRAM_ATTR _zh_isr_handler(void *arg) { BaseType_t xHigherPriorityTaskWoken = pdFALSE;