fix(esp_adc): fixed adc continue monitor don't work issue

Closes #14769
Closes #14814

components/esp_adc/adc_continuous.c

@@ -294,6 +294,22 @@ esp_err_t adc_continuous_start(adc_continuous_handle_t handle)
     }
 #endif  //#if SOC_ADC_ARBITER_SUPPORTED
 
+#if SOC_ADC_MONITOR_SUPPORTED
+    adc_ll_digi_monitor_clear_intr();
+    for (int i = 0; i < SOC_ADC_DIGI_MONITOR_NUM; i++) {
+        adc_monitor_t *monitor_ctx = handle->adc_monitor[i];
+        if (monitor_ctx) {
+            // config monitor hardware
+            adc_hal_digi_monitor_set_thres(monitor_ctx->monitor_id, monitor_ctx->config.adc_unit, monitor_ctx->config.channel, monitor_ctx->config.h_threshold, monitor_ctx->config.l_threshold);
+            // if monitor not enabled now, just using monitor api later
+            if (monitor_ctx->fsm == ADC_MONITOR_FSM_ENABLED) {
+                // restore the started FSM
+                adc_ll_digi_monitor_user_start(monitor_ctx->monitor_id, ((monitor_ctx->config.h_threshold >= 0) || (monitor_ctx->config.l_threshold >= 0)));
+            }
+        }
+    }
+#endif  //#if SOC_ADC_MONITOR_SUPPORTED
+
     if (handle->use_adc1) {
         adc_hal_set_controller(ADC_UNIT_1, ADC_HAL_CONTINUOUS_READ_MODE);
     }

components/esp_adc/adc_monitor.c

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -173,10 +173,6 @@ esp_err_t adc_new_continuous_monitor(adc_continuous_handle_t handle, const adc_m
         ESP_GOTO_ON_ERROR(adc_monitor_intr_alloc(), intr_err, MNTOR_TAG, "esp intr alloc failed");
     }
 
-    // config hardware
-    adc_ll_digi_monitor_clear_intr();
-    adc_ll_digi_monitor_set_thres(monitor_ctx->monitor_id, monitor_ctx->config.adc_unit, monitor_ctx->config.channel, monitor_ctx->config.h_threshold, monitor_ctx->config.l_threshold);
-
     *ret_handle = monitor_ctx;
     return ESP_OK;
 
@@ -191,7 +187,6 @@ esp_err_t adc_continuous_monitor_register_event_callbacks(adc_monitor_handle_t m
 {
     ESP_RETURN_ON_FALSE(monitor_handle && cbs, ESP_ERR_INVALID_ARG, MNTOR_TAG, "invalid argument: null pointer");
     ESP_RETURN_ON_FALSE(monitor_handle->fsm == ADC_MONITOR_FSM_INIT, ESP_ERR_INVALID_STATE, MNTOR_TAG, "monitor should be in init state");
-    ESP_RETURN_ON_FALSE(!(monitor_handle->cbs.on_over_high_thresh || monitor_handle->cbs.on_below_low_thresh), ESP_ERR_INVALID_STATE, MNTOR_TAG, "callbacks had been registered");
 #if CONFIG_IDF_TARGET_ESP32S2
     ESP_RETURN_ON_FALSE(!(cbs->on_below_low_thresh && cbs->on_over_high_thresh), ESP_ERR_NOT_SUPPORTED, MNTOR_TAG, "ESP32S2 support only one threshold");
 #endif
@@ -216,45 +211,31 @@ esp_err_t adc_continuous_monitor_register_event_callbacks(adc_monitor_handle_t m
 
 esp_err_t adc_continuous_monitor_enable(adc_monitor_handle_t monitor_handle)
 {
-    ESP_RETURN_ON_FALSE(monitor_handle, ESP_ERR_INVALID_ARG, MNTOR_TAG, "invalid argument: null pointer");
-    ESP_RETURN_ON_FALSE(monitor_handle->fsm == ADC_MONITOR_FSM_INIT, ESP_ERR_INVALID_STATE, MNTOR_TAG, "monitor should be in init state");
-
-    // enable peripheral intr_ena
-    if ((monitor_handle->config.h_threshold >= 0)) {
-        adc_ll_digi_monitor_enable_intr(monitor_handle->monitor_id, ADC_MONITOR_MODE_HIGH, true);
-    }
-    if ((monitor_handle->config.l_threshold >= 0)) {
-        adc_ll_digi_monitor_enable_intr(monitor_handle->monitor_id, ADC_MONITOR_MODE_LOW, true);
-    }
+    ESP_RETURN_ON_FALSE_ISR(monitor_handle, ESP_ERR_INVALID_ARG, MNTOR_TAG, "invalid argument: null pointer");
+    ESP_RETURN_ON_FALSE_ISR(monitor_handle->fsm == ADC_MONITOR_FSM_INIT, ESP_ERR_INVALID_STATE, MNTOR_TAG, "monitor should be in init state");
 
-    adc_ll_digi_monitor_user_start(monitor_handle->monitor_id, true);
+    // start monitor
+    adc_ll_digi_monitor_user_start(monitor_handle->monitor_id, ((monitor_handle->config.h_threshold >= 0) || (monitor_handle->config.l_threshold >= 0)));
     monitor_handle->fsm = ADC_MONITOR_FSM_ENABLED;
-    return esp_intr_enable(s_adc_monitor_platform.monitor_intr_handle);
+    return ESP_OK;
 }
 
 esp_err_t adc_continuous_monitor_disable(adc_monitor_handle_t monitor_handle)
 {
-    ESP_RETURN_ON_FALSE(monitor_handle, ESP_ERR_INVALID_ARG, MNTOR_TAG, "invalid argument: null pointer");
-    ESP_RETURN_ON_FALSE(monitor_handle->fsm == ADC_MONITOR_FSM_ENABLED, ESP_ERR_INVALID_STATE, MNTOR_TAG, "monitor not in running");
-
-    // disable peripheral intr_ena
-    if ((monitor_handle->config.h_threshold >= 0)) {
-        adc_ll_digi_monitor_enable_intr(monitor_handle->monitor_id, ADC_MONITOR_MODE_HIGH, false);
-    }
-    if ((monitor_handle->config.l_threshold >= 0)) {
-        adc_ll_digi_monitor_enable_intr(monitor_handle->monitor_id, ADC_MONITOR_MODE_LOW, false);
-    }
+    ESP_RETURN_ON_FALSE_ISR(monitor_handle, ESP_ERR_INVALID_ARG, MNTOR_TAG, "invalid argument: null pointer");
+    ESP_RETURN_ON_FALSE_ISR(monitor_handle->fsm == ADC_MONITOR_FSM_ENABLED, ESP_ERR_INVALID_STATE, MNTOR_TAG, "monitor not in running");
 
+    // stop monitor
     adc_ll_digi_monitor_user_start(monitor_handle->monitor_id, false);
     monitor_handle->fsm = ADC_MONITOR_FSM_INIT;
-    return esp_intr_disable(s_adc_monitor_platform.monitor_intr_handle);
+    return ESP_OK;
 }
 
 esp_err_t adc_del_continuous_monitor(adc_monitor_handle_t monitor_handle)
 {
     ESP_RETURN_ON_FALSE(monitor_handle, ESP_ERR_INVALID_ARG, MNTOR_TAG, "invalid argument: null pointer");
     ESP_RETURN_ON_FALSE((monitor_handle->fsm == ADC_MONITOR_FSM_INIT) && (s_adc_monitor_platform.continuous_ctx->fsm == ADC_FSM_INIT), \
-                        ESP_ERR_INVALID_STATE, MNTOR_TAG, "monitor and ADC continuous driver should all be in init state");
+                        ESP_ERR_INVALID_STATE, MNTOR_TAG, "monitor and ADC continuous driver should all stopped");
     ESP_RETURN_ON_ERROR(s_adc_monitor_release(monitor_handle), MNTOR_TAG, "monitor not find or isn't in use");
 
     for (int i = 0; i < SOC_ADC_DIGI_MONITOR_NUM; i++) {
@@ -265,7 +246,7 @@ esp_err_t adc_del_continuous_monitor(adc_monitor_handle_t monitor_handle)
         }
     }
 
-    // If no monitor is using, the release intr handle as well
+    // If no monitor is using, then release intr handle as well
     ESP_RETURN_ON_ERROR(esp_intr_free(s_adc_monitor_platform.monitor_intr_handle), MNTOR_TAG, "esp intr release failed\n");
     s_adc_monitor_platform.monitor_intr_handle = NULL;
     free(monitor_handle);

components/esp_adc/include/esp_adc/adc_monitor.h

@@ -65,7 +65,8 @@ typedef struct {
 esp_err_t adc_new_continuous_monitor(adc_continuous_handle_t handle, const adc_monitor_config_t *monitor_cfg, adc_monitor_handle_t *ret_handle);
 
 /**
- * @brief Register threshold interrupt callbacks for allocated monitor.
+ * @brief Register/Unregister threshold interrupt callbacks for allocated monitor.
+ *        Passing `cbs` contain the NULL `over_high/below_low` will unregister relative callbacks.
  *
  * @param[in]  monitor_handle       Monitor handle
  * @param[in]  cbs                  Pointer to a adc_monitor_evt_cbs_t struct

components/esp_adc/test_apps/adc/main/test_adc.c

@@ -382,9 +382,6 @@ TEST_CASE("ADC continuous monitor init_deinit", "[adc]")
     TEST_ESP_OK(adc_del_continuous_monitor(monitor_handle_2));
     TEST_ESP_ERR(ESP_ERR_INVALID_ARG, adc_del_continuous_monitor(monitor_handle_3));
 
-    //try register cbs again
-    TEST_ESP_ERR(ESP_ERR_INVALID_STATE, adc_continuous_monitor_register_event_callbacks(monitor_handle, &monitor_cb, &monitor_cb));
-
     //try delete it when adc is running but monitor not running
     TEST_ESP_OK(adc_continuous_start(handle));
     TEST_ESP_ERR(ESP_ERR_INVALID_STATE, adc_del_continuous_monitor(monitor_handle));
@@ -400,23 +397,8 @@ TEST_CASE("ADC continuous monitor init_deinit", "[adc]")
     TEST_ESP_OK(adc_continuous_deinit(handle));
 }
 
-/**
- * NOTE: To run this special feature test case, you need wire ADC channel pin you want to monit
- *       to a wave output pin defined below.
- *
- *       +---------+
- *       |         |
- *       |    (adc)|------------+
- *       |         |            |
- *       |   (wave)|------------+
- *       |         |
- *       |  ESP32  |
- *       +---------+
- *
- *       or you can connect your signals from signal generator to ESP32 pin which you monitoring
- **/
-#define TEST_ADC_CHANNEL    ADC_CHANNEL_0   //GPIO_1
-#define TEST_WAVE_OUT_PIN   GPIO_NUM_2      //GPIO_2
+#if SOC_ADC_CALIBRATION_V1_SUPPORTED
+#define TEST_ADC_CHANNEL    ADC_CHANNEL_0
 static uint32_t m1h_cnt, m1l_cnt;
 
 bool IRAM_ATTR m1h_cb(adc_monitor_handle_t monitor_handle, const adc_monitor_evt_data_t *event_data, void *user_data)
@@ -429,7 +411,7 @@ bool IRAM_ATTR m1l_cb(adc_monitor_handle_t monitor_handle, const adc_monitor_evt
     m1l_cnt ++;
     return false;
 }
-TEST_CASE("ADC continuous monitor functionary", "[adc][manual][ignore]")
+TEST_CASE("ADC continuous monitor functionary", "[adc]")
 {
     adc_continuous_handle_t handle = NULL;
     adc_continuous_handle_cfg_t adc_config = {
@@ -462,9 +444,9 @@ TEST_CASE("ADC continuous monitor functionary", "[adc][manual][ignore]")
 #if CONFIG_IDF_TARGET_ESP32S2
         .h_threshold = -1,      //S2 support only one threshold for one monitor
 #else
-        .h_threshold = 3000,
+        .h_threshold = ADC_TEST_HIGH_VAL_DMA - (ADC_TEST_HIGH_VAL_DMA - ADC_TEST_LOW_VAL) / 4,
 #endif
-        .l_threshold = 1000,
+        .l_threshold = ADC_TEST_LOW_VAL + (ADC_TEST_HIGH_VAL_DMA - ADC_TEST_LOW_VAL) / 4,
     };
     adc_monitor_evt_cbs_t monitor_cb = {
 #if !CONFIG_IDF_TARGET_ESP32S2
@@ -474,41 +456,46 @@ TEST_CASE("ADC continuous monitor functionary", "[adc][manual][ignore]")
     };
     TEST_ESP_OK(adc_new_continuous_monitor(handle, &adc_monitor_cfg, &monitor_handle));
     TEST_ESP_OK(adc_continuous_monitor_register_event_callbacks(monitor_handle, &monitor_cb, NULL));
-
-    //config a pin to generate wave
-    gpio_config_t gpio_cfg = {
-        .pin_bit_mask = (1ULL << TEST_WAVE_OUT_PIN),
-        .mode = GPIO_MODE_INPUT_OUTPUT,
-        .pull_up_en = GPIO_PULLDOWN_ENABLE,
-    };
-    TEST_ESP_OK(gpio_config(&gpio_cfg));
-
     TEST_ESP_OK(adc_continuous_monitor_enable(monitor_handle));
     TEST_ESP_OK(adc_continuous_start(handle));
 
-    for (uint8_t i = 0; i < 8; i++) {
-        vTaskDelay(1000);
+    int adc_io;
+    adc_continuous_channel_to_io(ADC_UNIT_1, TEST_ADC_CHANNEL, &adc_io);
+    printf("Using ADC_CHANNEL_%d on GPIO%d\n", TEST_ADC_CHANNEL, adc_io);
 
-        // check monitor cb
-        printf("%d\t high_cnt %4ld\tlow_cnt %4ld\n", i, m1h_cnt, m1l_cnt);
-        if (gpio_get_level(TEST_WAVE_OUT_PIN)) {
+    // Test with internal gpio pull up/down, detail and order refer to `gpio_pull_mode_t`
+    // pull_up + pull_down to get half ADC convert
+    for (uint8_t i = 0; i < 8; i++) {
+        int pull_mode = i % GPIO_FLOATING;
+        gpio_set_pull_mode(adc_io, pull_mode);
+        vTaskDelay(10); // wait some time for GPIO level be stable
+        m1h_cnt = 0;
+        m1l_cnt = 0;
+        vTaskDelay(1000);   //time to count monitor interrupt
+        printf("%d\t %s\t high_cnt %4ld\tlow_cnt %4ld\n", i, (pull_mode == 0) ? "up  " : (pull_mode == 1) ? "down" : "mid ", m1h_cnt, m1l_cnt);
+
+        switch (pull_mode) {
+        case GPIO_PULLUP_ONLY:
 #if !CONFIG_IDF_TARGET_ESP32S2
-            // TEST_ASSERT_UINT32_WITHIN(SOC_ADC_SAMPLE_FREQ_THRES_LOW*0.1, SOC_ADC_SAMPLE_FREQ_THRES_LOW, m1h_cnt);
-            // TEST_ASSERT_LESS_THAN_UINT32(5, m1l_cnt);   //Actually, it will still encountered 1~2 times because hardware run very quickly
+            TEST_ASSERT_UINT32_WITHIN(SOC_ADC_SAMPLE_FREQ_THRES_LOW * 0.1, SOC_ADC_SAMPLE_FREQ_THRES_LOW, m1h_cnt);
+            TEST_ASSERT_EQUAL(0, m1l_cnt);  //low limit should NOT triggrted when pull_up
 #endif
-            m1h_cnt = 0;
-            gpio_set_level(TEST_WAVE_OUT_PIN, 0);
-        } else {
+            break;
+        case GPIO_PULLDOWN_ONLY:
             TEST_ASSERT_UINT32_WITHIN(SOC_ADC_SAMPLE_FREQ_THRES_LOW * 0.1, SOC_ADC_SAMPLE_FREQ_THRES_LOW, m1l_cnt);
-            TEST_ASSERT_LESS_THAN_UINT32(5, m1h_cnt);   //Actually, it will still encountered 1~2 times because hardware run very quickly
-            m1l_cnt = 0;
-            gpio_set_level(TEST_WAVE_OUT_PIN, 1);
+            TEST_ASSERT_EQUAL(0, m1h_cnt);
+            break;
+        case GPIO_PULLUP_PULLDOWN:
+            TEST_ASSERT_EQUAL(0, m1h_cnt);  //half votage, both limit should NOT triggered
+            TEST_ASSERT_EQUAL(0, m1l_cnt);
+            break;
+        default: printf("unknown gpio pull mode !!!\n");
         }
     }
     TEST_ESP_OK(adc_continuous_stop(handle));
     TEST_ESP_OK(adc_continuous_monitor_disable(monitor_handle));
     TEST_ESP_OK(adc_del_continuous_monitor(monitor_handle));
     TEST_ESP_OK(adc_continuous_deinit(handle));
 }
-
+#endif  //SOC_ADC_CALIBRATION_V1_SUPPORTED
 #endif  //SOC_ADC_MONITOR_SUPPORTED && CONFIG_SOC_ADC_DMA_SUPPORTED

components/hal/adc_hal.c

@@ -300,3 +300,14 @@ void adc_hal_digi_clr_eof(void)
     adc_ll_digi_dma_clr_eof();
 }
 #endif
+
+#if SOC_ADC_MONITOR_SUPPORTED
+void adc_hal_digi_monitor_set_thres(adc_monitor_id_t monitor_id, adc_unit_t adc_n, uint8_t adc_ch, int32_t h_thres, int32_t l_thres)
+{
+    adc_ll_digi_monitor_set_thres(monitor_id, adc_n, adc_ch, h_thres, l_thres);
+
+    // enable peripheral intr_ena accordingly
+    adc_ll_digi_monitor_enable_intr(monitor_id, ADC_MONITOR_MODE_HIGH, (h_thres >= 0));
+    adc_ll_digi_monitor_enable_intr(monitor_id, ADC_MONITOR_MODE_LOW, (l_thres >= 0));
+}
+#endif  //SOC_ADC_MONITOR_SUPPORTED

components/hal/include/hal/adc_hal.h

@@ -130,7 +130,7 @@ void adc_hal_dma_ctx_config(adc_hal_dma_ctx_t *hal, const adc_hal_dma_config_t *
  * Setting the digital controller.
  *
  * @param hal    Context of the HAL
- * @param cfg    Pointer to digital controller paramter.
+ * @param cfg    Pointer to digital controller parameter.
  */
 void adc_hal_digi_controller_config(adc_hal_dma_ctx_t *hal, const adc_hal_digi_ctrlr_cfg_t *cfg);
 
@@ -180,6 +180,17 @@ void adc_hal_digi_reset(void);
 void adc_hal_digi_clr_eof(void);
 #endif
 
+/**
+ * @brief Set ADC monitor with high and low thresholds, and will enable the interrupts accordingly
+ *
+ * @param monitor_id Monitor to configure
+ * @param adc_n Which ADC unit will be monitored
+ * @param adc_ch Which ADC channel will be monitored
+ * @param h_thres High threshold (disable if < 0)
+ * @param l_thres Low threshold (disable if < 0)
+ */
+void adc_hal_digi_monitor_set_thres(adc_monitor_id_t monitor_id, adc_unit_t adc_n, uint8_t adc_ch, int32_t h_thres, int32_t l_thres);
+
 #ifdef __cplusplus
 }
 #endif