wip:
This commit is contained in:
		
							
								
								
									
										302
									
								
								gt.h
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										302
									
								
								gt.h
									
									
									
									
									
										Normal file
									
								
							| @@ -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. **Меньше дублирования:** Логика проверки аргументов и результатов вызовов вынесена в макросы. | ||||
		Reference in New Issue
	
	Block a user