led_indicator: update ledc driver

Resolve #299

components/led/led_indicator/CHANGELOG.md

@@ -1,5 +1,15 @@
 # ChangeLog
 
+## v0.5.0 - 2023-10-7
+
+### Enhancements:
+
+* Simplified the initialization process for LEDC.
+
+### BUG FIX
+
+* Resolved the blink issue in the breathing mode.
+
 ## v0.4.0 - 2023-4-12
 
 ### Enhancements:

components/led/led_indicator/Kconfig

@@ -9,4 +9,38 @@ menu "LED Indicator"
         bool "Enable gamma correction"
         default "y"
 
+    menu "LEDC Config"
+
+        choice LEDC_SPEED_MODE
+            prompt "LEDC Speed Mode"
+            default LEDC_LOW_SPEED_MODE
+
+        config LEDC_HIGH_SPEED_MODE
+            bool "LEDC high speed mode"
+            depends on SOC_LEDC_SUPPORT_HS_MODE
+        config LEDC_LOW_SPEED_MODE
+            bool "LEDC low speed mode"
+        endchoice
+
+        config LEDC_SPEED_MODE_VALUE
+            int
+            default 1 if LEDC_LOW_SPEED_MODE && SOC_LEDC_SUPPORT_HS_MODE
+            default 0 if LEDC_HIGH_SPEED_MODE && SOC_LEDC_SUPPORT_HS_MODE
+            default 0 if LEDC_LOW_SPEED_MODE && !SOC_LEDC_SUPPORT_HS_MODE
+
+        config LEDC_TIMER_BIT_NUM
+            int "LEDC Timer Bit Num"
+            default 13
+            range 1 20 if SOC_LEDC_TIMER_BIT_WIDE_NUM > 14
+            range 1 14 if SOC_LEDC_TIMER_BIT_WIDE_NUM <= 14
+            help
+                The bit number of LEDC timer (1 - SOC_LEDC_TIMER_BIT_WIDE_NUM)
+
+        config LEDC_TIMER_FREQ_HZ
+            int "LEDC Timer Frequency"
+            default 5000
+            help
+                The frequency of LEDC timer (1 - 40000000)
+    endmenu
+
 endmenu

components/led/led_indicator/idf_component.yml

@@ -1,4 +1,4 @@
-version: "0.4.0"
+version: "0.5.0"
 description: LED indicator driver
 url: https://github.com/espressif/esp-iot-solution/tree/master/components/led/led_indicator
 dependencies:

components/led/led_indicator/include/led_custom.h

@@ -42,10 +42,10 @@ typedef enum {
  *
  */
 typedef struct {
-    bool is_active_level_high;                                                             /**< Set true if GPIO level is high when light is ON, otherwise false. */
+    bool is_active_level_high;                                                             /*!< Set true if GPIO level is high when light is ON, otherwise false. */
     led_indicator_duty_t duty_resolution;                                                  /*!< Resolution of duty setting in number of bits. The range of duty values is [0, (2**duty_resolution) -1]. If the brightness cannot be set, set this as 1. */
     esp_err_t (*hal_indicator_init)(void *hardware_data );                                 /*!< pointer functions for initialization*/
-    esp_err_t (*hal_indicator_set_on_off)(void *hardware_data, bool on_off);                /*!< pointer functions for setting on or off */
+    esp_err_t (*hal_indicator_set_on_off)(void *hardware_data, bool on_off);               /*!< pointer functions for setting on or off */
     esp_err_t (*hal_indicator_deinit)(void *hardware_data );                               /*!< pointer functions for deinitialization */
     esp_err_t (*hal_indicator_set_brightness)(void *hardware_data, uint32_t brightness);   /*!< pointer functions for setting brightness, must be supported by hardware */
     void *hardware_data;                                                                   /*!< user hardware data*/

components/led/led_indicator/include/led_gpio.h

@@ -17,8 +17,8 @@ extern "C" {
  *
  */
 typedef struct {
-    bool is_active_level_high;     /**< Set true if GPIO level is high when light is ON, otherwise false. */
-    int32_t gpio_num;              /**< num of GPIO */
+    bool is_active_level_high;     /*!< Set true if GPIO level is high when light is ON, otherwise false. */
+    int32_t gpio_num;              /*!< num of GPIO */
 } led_indicator_gpio_config_t;
 
 /**

components/led/led_indicator/include/led_indicator.h

@@ -22,11 +22,11 @@ extern "C" {
  *
  */
 enum {
-    LED_STATE_OFF = 0,          /*!< turn off the LED */
-    LED_STATE_25_PERCENT = 64,  /*!< 25% brightness, must support to set brightness */
+    LED_STATE_OFF = 0,           /*!< turn off the LED */
+    LED_STATE_25_PERCENT = 64,   /*!< 25% brightness, must support to set brightness */
     LED_STATE_50_PERCENT = 128,  /*!< 50% brightness, must support to set brightness */
     LED_STATE_75_PERCENT = 191,  /*!< 75% brightness, must support to set brightness */
-    LED_STATE_ON = UINT8_MAX,   /*!< turn on the LED */
+    LED_STATE_ON = UINT8_MAX,    /*!< turn on the LED */
 };
 
 /**

components/led/led_indicator/include/led_ledc.h

@@ -12,10 +12,16 @@
 extern "C" {
 #endif
 
+#define LEDC_MODE       CONFIG_LEDC_SPEED_MODE_VALUE
+#define LEDC_DUTY_RES   CONFIG_LEDC_TIMER_BIT_NUM
+#define LEDC_FREQ_HZ    CONFIG_LEDC_TIMER_FREQ_HZ
+
 typedef struct {
-    bool is_active_level_high;                     /**< Set true if GPIO level is high when light is ON, otherwise false. */
-    ledc_timer_config_t *ledc_timer_config;        /*!< Configuration parameters of LEDC timer for ledc_timer_config function*/
-    ledc_channel_config_t *ledc_channel_config;    /*!< Configuration parameters of LEDC channel for ledc_channel_config function*/
+    bool is_active_level_high;              /*!< Set true if GPIO level is high when light is ON, otherwise false. */
+    bool timer_inited;                      /*!< Set true if LEDC timer is inited, otherwise false. */
+    ledc_timer_t timer_num;                 /*!< The timer source of channel */
+    int32_t gpio_num;                       /*!< num of gpio */
+    ledc_channel_t channel;                 /*!< LEDC channel */
 } led_indicator_ledc_config_t;
 
 /**

components/led/led_indicator/src/led_indicator.c

@@ -45,7 +45,7 @@ static const char *led_indicator_mode_str[3] = {"GPIO mode", "LEDC mode", "custo
  *
  */
 typedef struct {
-    bool is_active_level_high;                                                            /**< Set true if GPIO level is high when light is ON, otherwise false. */
+    bool is_active_level_high;                                                            /*!< Set true if GPIO level is high when light is ON, otherwise false. */
     esp_err_t (*hal_indicator_set_on_off)(void *hardware_data, bool on_off);              /*!< Pointer function for setting on or off */
     esp_err_t (*hal_indicator_deinit)(void *hardware_data);                               /*!< Pointer function for Deinitialization */
     esp_err_t (*hal_indicator_set_brightness)(void *hardware_data, uint32_t brightness);  /*!< Pointer function for setting brightness, must be supported by hardware */
@@ -54,9 +54,9 @@ typedef struct {
     int active_blink;                  /*!< Active blink list*/
     int preempt_blink;                 /*!< Highest priority blink list*/
     int *p_blink_steps;                /*!< Stage of each blink list */
-    uint8_t fade_value_count;          /*!< Count the number of fade */
     uint8_t current_fade_value;        /*!< Current fade value */
     uint8_t last_fade_value;           /*!< Save the last brightness value */
+    uint16_t fade_value_count;         /*!< Count the number of fade */
     uint32_t max_duty;                 /*!< Max duty cycle from duty_resolution : 2^duty_resolution -1 */
     SemaphoreHandle_t mutex;           /*!< Mutex to achieve thread-safe */
     TimerHandle_t h_timer;             /*!< LED timer handle, invalid if works in pwm mode */
@@ -70,7 +70,7 @@ typedef struct _led_indicator_slist_t {
 } _led_indicator_slist_t;
 
 typedef struct _led_indicator_com_config {
-    bool is_active_level_high;                                                            /**< Set true if GPIO level is high when light is ON, otherwise false. */
+    bool is_active_level_high;                                                            /*!< Set true if GPIO level is high when light is ON, otherwise false. */
     esp_err_t (*hal_indicator_set_on_off)(void *hardware_data, bool on_off);              /*!< Pointer function for setting on or off */
     esp_err_t (*hal_indicator_deinit)(void *hardware_data);                               /*!< Pointer function for Deinitialization */
     esp_err_t (*hal_indicator_set_brightness)(void *hardware_data, uint32_t brightness);  /*!< Pointer function for setting brightness, must be supported by hardware */
@@ -191,6 +191,7 @@ static void _blink_list_runner(TimerHandle_t xTimer)
 
         case LED_BLINK_BRIGHTNESS: {
             if (!p_led_indicator->hal_indicator_set_brightness) {
+                ESP_LOGW(TAG, "LED indicator does not have the hal_indicator_set_brightness function");
                 break;
             }
 
@@ -212,6 +213,7 @@ static void _blink_list_runner(TimerHandle_t xTimer)
 
         case LED_BLINK_BREATHE: {
             if (!p_led_indicator->hal_indicator_set_brightness) {
+                ESP_LOGW(TAG, "LED indicator does not have the hal_indicator_set_brightness function");
                 break;
             }
 
@@ -291,7 +293,6 @@ static _led_indicator_t *_led_indicator_create_com(_led_indicator_com_config_t *
     p_led_indicator->p_blink_steps = (int *)calloc(cfg->blink_list_num, sizeof(int));
     LED_INDICATOR_CHECK_WARNING(p_led_indicator->hal_indicator_set_on_off != NULL, "LED indicator does not have the hal_indicator_set_on_off function",);
     LED_INDICATOR_CHECK_WARNING(p_led_indicator->hal_indicator_deinit != NULL, "LED indicator does not have the hal_indicator_deinit function",);
-    LED_INDICATOR_CHECK_WARNING(p_led_indicator->hal_indicator_set_brightness != NULL, "LED indicator does not have the hal_indicator_set_brightness function",);
     LED_INDICATOR_CHECK_WARNING(p_led_indicator->p_blink_steps != NULL, "calloc blink_steps memory failed", goto cleanup_indicator);
     for (size_t j = 0; j < cfg->blink_list_num; j++) {
         *(p_led_indicator->p_blink_steps + j) = LED_BLINK_STOP;
@@ -358,11 +359,11 @@ led_indicator_handle_t led_indicator_create(const led_indicator_config_t *config
         ret = led_indicator_ledc_init((void *)cfg);
         LED_INDICATOR_CHECK(ESP_OK == ret, "LEDC mode init failed", return NULL);
         com_cfg.is_active_level_high = cfg->is_active_level_high;
-        com_cfg.hardware_data = (void *)cfg->ledc_channel_config->channel;
+        com_cfg.hardware_data = (void *)cfg->channel;
         com_cfg.hal_indicator_set_on_off = led_indicator_ledc_set_on_off;
         com_cfg.hal_indicator_deinit = led_indicator_ledc_deinit;
         com_cfg.hal_indicator_set_brightness = led_indicator_ledc_set_brightness;
-        com_cfg.duty_resolution = cfg->ledc_timer_config->duty_resolution;
+        com_cfg.duty_resolution = LEDC_DUTY_RES;
         if (config->blink_lists == NULL) {
             ESP_LOGI(TAG, "blink_lists is null, use default blink list");
             com_cfg.blink_lists = default_led_indicator_blink_lists;
@@ -380,6 +381,7 @@ led_indicator_handle_t led_indicator_create(const led_indicator_config_t *config
         const led_indicator_custom_config_t *cfg = config->led_indicator_custom_config;
         LED_INDICATOR_CHECK_WARNING(cfg->hal_indicator_init != NULL, "LED indicator does not have hal_indicator_init ", goto without_init);
         ret = cfg->hal_indicator_init(cfg->hardware_data);
+        LED_INDICATOR_CHECK(ret == ESP_OK, "LED indicator init failed", return NULL);
 without_init:
         com_cfg.is_active_level_high = cfg->is_active_level_high;
         com_cfg.hardware_data = cfg->hardware_data;

components/led/led_indicator/src/led_ledc.c

@@ -20,14 +20,32 @@
 
 #define LEDC_MAX_CHANNEL 8
 
+#define LEDC_TIMER_CONFIG(ledc_timer)   \
+{                                     \
+    .speed_mode = LEDC_MODE,          \
+    .duty_resolution = LEDC_DUTY_RES, \
+    .timer_num = ledc_timer,          \
+    .freq_hz = LEDC_FREQ_HZ,          \
+    .clk_cfg = LEDC_AUTO_CLK,         \
+}
+
+#define LEDC_CHANNEL_CONFIG(ledc_timer, ledc_channel, gpio) \
+{                                                       \
+    .speed_mode = LEDC_MODE,                            \
+    .timer_sel = ledc_timer,                            \
+    .hpoint = 0,                                        \
+    .duty = 0,                                          \
+    .intr_type = LEDC_INTR_DISABLE,                     \
+    .channel = ledc_channel,                            \
+    .gpio_num = gpio,                                   \
+}
+
 typedef struct {
     bool is_init;                   /*!< Is the channel being used */
-    ledc_mode_t speed_mode;         /*!< LEDC speed speed_mode, high-speed mode or low-speed mode */
     ledc_channel_t channel;         /*!< LEDC channel (0 - 7) */
 } led_indicator_ledc_channel_t;
 
 typedef struct {
-    bool ledc_timer_is_init;                                      /*!< Avoid repeated init ledc_timer */
     uint8_t channel_num;                                          /*!< Number of LEDC channels already in use */
     uint32_t max_duty;                                            /*!< Max duty cycle from duty_resolution : 2^duty_resolution -1 */
     led_indicator_ledc_channel_t ledc_channel[LEDC_MAX_CHANNEL];  /*!< LEDC channel state */
@@ -38,26 +56,25 @@ static led_indicator_ledc_t *s_ledc = NULL;
 esp_err_t led_indicator_ledc_init(void *param)
 {
     esp_err_t ret = ESP_OK;
-    uint32_t ch;
     const led_indicator_ledc_config_t *cfg = (led_indicator_ledc_config_t *)param;
     if (!s_ledc) {
         s_ledc = calloc(1, sizeof(led_indicator_ledc_t));
     }
     LED_LEDC_CHECK(NULL != s_ledc, "alloc fail", return ESP_ERR_NO_MEM);
 
-    if ( !s_ledc->ledc_timer_is_init ) {
-        ret = ledc_timer_config(cfg->ledc_timer_config);
+    if (!cfg->timer_inited) {
+        ledc_timer_config_t ledc_timer_cfg = LEDC_TIMER_CONFIG(cfg->timer_num); 
+        ret = ledc_timer_config(&ledc_timer_cfg);
         LED_LEDC_CHECK(ESP_OK == ret, "LEDC timer config fail!", goto EXIT);
-        s_ledc->ledc_timer_is_init = true;
-        s_ledc->max_duty = pow(2, cfg->ledc_timer_config->duty_resolution) - 1;
+        s_ledc->max_duty = pow(2, ledc_timer_cfg.duty_resolution) - 1;
     }
 
-    ch = cfg->ledc_channel_config->channel;
+    ledc_channel_t ch = cfg->channel;
     LED_LEDC_CHECK(!s_ledc->ledc_channel[ch].is_init, "LEDC channel is already initialized!", goto EXIT);
-    ret = ledc_channel_config(cfg->ledc_channel_config);
+    ledc_channel_config_t ledc_ch_cfg = LEDC_CHANNEL_CONFIG(cfg->timer_num, cfg->channel, cfg->gpio_num);
+    ret = ledc_channel_config(&ledc_ch_cfg);
     LED_LEDC_CHECK(ESP_OK == ret, "ledc_channel_config fail!", goto EXIT);
-    s_ledc->ledc_channel[ch].speed_mode = cfg->ledc_channel_config->speed_mode;
-    s_ledc->ledc_channel[ch].channel = cfg->ledc_channel_config->channel;
+    s_ledc->ledc_channel[ch].channel = ch;
     s_ledc->ledc_channel[ch].is_init = true;
     s_ledc->channel_num += 1;
     return ESP_OK;
@@ -73,6 +90,7 @@ esp_err_t led_indicator_ledc_init(void *param)
 esp_err_t led_indicator_ledc_deinit(void *channel)
 {
     uint32_t ch = (uint32_t)channel;
+    LED_LEDC_CHECK(NULL != s_ledc, "LEDC is not initialized", return ESP_FAIL);
     LED_LEDC_CHECK(s_ledc->ledc_channel[ch].is_init, "LEDC channel does't init", return ESP_FAIL);
     s_ledc->ledc_channel[ch].is_init = false;
     s_ledc->channel_num -= 1;
@@ -90,14 +108,14 @@ esp_err_t led_indicator_ledc_set_on_off(void *channel, bool on_off)
     uint32_t ch = (uint32_t)channel;
     LED_LEDC_CHECK(s_ledc->ledc_channel[ch].is_init, "LEDC channel does't init", return ESP_FAIL);
     if (on_off) {
-        ret = ledc_set_duty(s_ledc->ledc_channel[ch].speed_mode, s_ledc->ledc_channel[ch].channel, s_ledc->max_duty);
+        ret = ledc_set_duty(LEDC_MODE, s_ledc->ledc_channel[ch].channel, s_ledc->max_duty);
         LED_LEDC_CHECK(ESP_OK == ret, "LEDC set duty error", return ret);
     } else {
-        ret = ledc_set_duty(s_ledc->ledc_channel[ch].speed_mode, s_ledc->ledc_channel[ch].channel, on_off);
+        ret = ledc_set_duty(LEDC_MODE, s_ledc->ledc_channel[ch].channel, on_off);
         LED_LEDC_CHECK(ESP_OK == ret, "LEDC set duty error", return ret);
     }
 
-    ret = ledc_update_duty(s_ledc->ledc_channel[ch].speed_mode, s_ledc->ledc_channel[ch].channel);
+    ret = ledc_update_duty(LEDC_MODE, s_ledc->ledc_channel[ch].channel);
     LED_LEDC_CHECK(ESP_OK == ret, "LEDC update duty error", return ret);
     return ESP_OK;
 }
@@ -108,9 +126,9 @@ esp_err_t led_indicator_ledc_set_brightness(void *channel, uint32_t brightness)
     uint32_t ch = (uint32_t)channel;
     LED_LEDC_CHECK(s_ledc->ledc_channel[ch].is_init, "LEDC channel does't init", return ESP_FAIL);
     LED_LEDC_CHECK(s_ledc->max_duty > brightness, "brightness can't be larger than (2^max_duty - 1)", return ESP_FAIL);
-    ret = ledc_set_duty(s_ledc->ledc_channel[ch].speed_mode, s_ledc->ledc_channel[ch].channel, brightness);
+    ret = ledc_set_duty(LEDC_MODE, s_ledc->ledc_channel[ch].channel, brightness);
     LED_LEDC_CHECK(ESP_OK == ret, "LEDC set duty error", return ret);
-    ret = ledc_update_duty(s_ledc->ledc_channel[ch].speed_mode, s_ledc->ledc_channel[ch].channel);
+    ret = ledc_update_duty(LEDC_MODE, s_ledc->ledc_channel[ch].channel);
     LED_LEDC_CHECK(ESP_OK == ret, "LEDC update duty error", return ret);
     return ESP_OK;
 }

components/led/led_indicator/test_apps/main/CMakeLists.txt

@@ -1,2 +1,2 @@
-idf_component_register(SRCS "led_indicator_test.c"
-                        REQUIRES led_indicator unity)
+idf_component_register(SRC_DIRS "." 
+                       INCLUDE_DIRS ".")

components/led/led_indicator/test_apps/main/idf_component.yml

@@ -0,0 +1,17 @@
+## IDF Component Manager Manifest File
+dependencies:
+  espressif/button: "*"
+  ## Required IDF version
+  idf:
+    version: ">=4.0.0"
+  # # Put list of dependencies here
+  # # For components maintained by Espressif:
+  # component: "~1.0.0"
+  # # For 3rd party components:
+  # username/component: ">=1.0.0,<2.0.0"
+  # username2/component2:
+  #   version: "~1.0.0"
+  #   # For transient dependencies `public` flag can be set.
+  #   # `public` flag doesn't have an effect dependencies of the `main` component.
+  #   # All dependencies of `main` are public by default.
+  #   public: true