From c0846f0bb702b3198ffa95ec3894d2e75aa0cfed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Hru=C5=A1ka?= Date: Mon, 16 Apr 2018 13:33:54 +0200 Subject: [PATCH] Fix incorrect ADC channel counting with 16,17. Also make averaging toggleable. This bug caused wrong measurements, DAC distortion and instability with 16+17 and at least 1 other channel. --- units/adc/_adc_core.c | 31 +++++++++++++++------- units/adc/_adc_init.c | 54 +++++++++++++++++++++------------------ units/adc/_adc_internal.h | 1 + units/adc/_adc_settings.c | 13 +++++++++- units/adc/unit_adc.c | 7 ++++- 5 files changed, 70 insertions(+), 36 deletions(-) diff --git a/units/adc/_adc_core.c b/units/adc/_adc_core.c index 5b3938e..30db8f8 100644 --- a/units/adc/_adc_core.c +++ b/units/adc/_adc_core.c @@ -327,8 +327,7 @@ void UADC_DMA_Handler(void *arg) */ void UADC_ADC_EOS_Handler(void *arg) { - GPIOC->BSRR = 0x01;//TODO remove - +// GPIOC->BSRR = 0x01; Unit *unit = arg; struct priv *priv = unit->data; @@ -341,11 +340,16 @@ void UADC_ADC_EOS_Handler(void *arg) goto exit; } +// GPIOC->BSRR = 0x02; // Wait for the DMA to complete copying the last sample uint32_t dmapos = DMA_POS(priv); - if ((DMA_POS(priv) % priv->nb_channels) != 0) { - hw_wait_while((dmapos = DMA_POS(priv)) % priv->nb_channels != 0, 100); // XXX this could be changed to reading it from the DR instead + uint32_t err = (dmapos % priv->nb_channels); + if (err != 0) { + GPIOC->BSRR = 0x02; + hw_wait_while(((dmapos = DMA_POS(priv)) % priv->nb_channels) != 0, 10); + GPIOC->BRR = 0x02; } +// GPIOC->BRR = 0x02; uint32_t sample_pos; if (dmapos == 0) { @@ -355,14 +359,22 @@ void UADC_ADC_EOS_Handler(void *arg) } sample_pos -= priv->nb_channels; - int cnt = 0; // index of the sample within the group + uint8_t cnt = 0; // index of the sample within the group - const bool can_average = priv->real_frequency_int < UADC_MAX_FREQ_FOR_AVERAGING; + const bool can_average = priv->cfg.enable_averaging && priv->real_frequency_int < UADC_MAX_FREQ_FOR_AVERAGING; const uint32_t channels_mask = priv->channels_mask; + uint16_t val; + // TODO change this to a pre-computed byte array traversal + for (uint8_t i = 0; i < 18; i++) { if (channels_mask & (1 << i)) { - const uint16_t val = priv->dma_buffer[sample_pos+cnt]; +// if (cnt == priv->nb_channels-1) { +// val = last_sample; // DMA may not have finished copying it yet +// } else { + val = priv->dma_buffer[sample_pos+cnt]; +// } + cnt++; if (can_average) { @@ -375,8 +387,9 @@ void UADC_ADC_EOS_Handler(void *arg) } } + // Triggering condition test if (priv->opmode == ADC_OPMODE_ARMED) { - const uint16_t val = priv->last_samples[priv->trigger_source]; + val = priv->last_samples[priv->trigger_source]; if ((priv->trig_prev_level < priv->trig_level) && val >= priv->trig_level && (bool) (priv->trig_edge & 0b01)) { // Rising edge @@ -401,7 +414,7 @@ void UADC_ADC_EOS_Handler(void *arg) } exit: - GPIOC->BRR = 0x01;//TODO remove +// GPIOC->BRR = 0x01; return; } diff --git a/units/adc/_adc_init.c b/units/adc/_adc_init.c index 59779ce..1e81ea5 100644 --- a/units/adc/_adc_init.c +++ b/units/adc/_adc_init.c @@ -21,6 +21,7 @@ error_t UADC_preInit(Unit *unit) priv->cfg.frequency = 1000; priv->cfg.buffer_size = 256; // in half-words priv->cfg.averaging_factor = 500; // 0.5 + priv->cfg.enable_averaging = true; priv->opmode = ADC_OPMODE_UNINIT; @@ -120,33 +121,36 @@ error_t UADC_init(Unit *unit) // Claim and configure all analog pins priv->nb_channels = 0; for (uint8_t i = 0; i <= UADC_MAX_CHANNEL; i++) { - if (priv->cfg.channels & (1 << i)) { + if (priv->cfg.channels & (1UL << i)) { priv->nb_channels++; - char c; - uint8_t num; - if (i <= 7) { - c = 'A'; - num = i; - } - else if (i <= 9) { - c = 'B'; - num = (uint8_t) (i - 8); - } - else if (i <= 15) { - c = 'C'; - num = (uint8_t) (i - 10); - } else { - break; - } - - TRY(rsc_claim_pin(unit, c, num)); - uint32_t ll_pin = hw_pin2ll(num, &suc); - GPIO_TypeDef *port = hw_port2periph(c, &suc); - assert_param(suc); - - LL_GPIO_SetPinPull(port, ll_pin, LL_GPIO_PULL_NO); - LL_GPIO_SetPinMode(port, ll_pin, LL_GPIO_MODE_ANALOG); + do { + char c; + uint8_t num; + if (i <= 7) { + c = 'A'; + num = i; + } + else if (i <= 9) { + c = 'B'; + num = (uint8_t) (i - 8); + } + else if (i <= 15) { + c = 'C'; + num = (uint8_t) (i - 10); + } + else { + break; + } + + TRY(rsc_claim_pin(unit, c, num)); + uint32_t ll_pin = hw_pin2ll(num, &suc); + GPIO_TypeDef *port = hw_port2periph(c, &suc); + assert_param(suc); + + LL_GPIO_SetPinPull(port, ll_pin, LL_GPIO_PULL_NO); + LL_GPIO_SetPinMode(port, ll_pin, LL_GPIO_MODE_ANALOG); + } while (0); } } diff --git a/units/adc/_adc_internal.h b/units/adc/_adc_internal.h index 31091d1..afa3a26 100644 --- a/units/adc/_adc_internal.h +++ b/units/adc/_adc_internal.h @@ -46,6 +46,7 @@ struct priv { uint32_t frequency; //!< Timer frequency in Hz. Note: not all frequencies can be achieved accurately uint32_t buffer_size; //!< Buffer size in bytes (count 2 bytes per channel per measurement) - faster sampling freq needs bigger buffer uint16_t averaging_factor; //!< Exponential averaging factor 0-1000 + bool enable_averaging; } cfg; // Peripherals diff --git a/units/adc/_adc_settings.c b/units/adc/_adc_settings.c index f48f31e..03b32ee 100644 --- a/units/adc/_adc_settings.c +++ b/units/adc/_adc_settings.c @@ -23,6 +23,10 @@ void UADC_loadBinary(Unit *unit, PayloadParser *pp) priv->cfg.frequency = pp_u32(pp); priv->cfg.buffer_size = pp_u32(pp); priv->cfg.averaging_factor = pp_u16(pp); + + if (version >= 1) { + priv->cfg.enable_averaging = pp_bool(pp); + } } /** Write to a binary buffer for storing in Flash */ @@ -30,13 +34,14 @@ void UADC_writeBinary(Unit *unit, PayloadBuilder *pb) { struct priv *priv = unit->data; - pb_u8(pb, 0); // version + pb_u8(pb, 1); // version pb_u32(pb, priv->cfg.channels); pb_u8(pb, priv->cfg.sample_time); pb_u32(pb, priv->cfg.frequency); pb_u32(pb, priv->cfg.buffer_size); pb_u16(pb, priv->cfg.averaging_factor); + pb_bool(pb, priv->cfg.enable_averaging); } // ------------------------------------------------------------------------ @@ -64,6 +69,9 @@ error_t UADC_loadIni(Unit *unit, const char *key, const char *value) priv->cfg.averaging_factor = cfg_u16_parse(value, &suc); if (priv->cfg.averaging_factor > 1000) return E_BAD_VALUE; } + else if (streq(key, "averaging")) { + priv->cfg.enable_averaging = cfg_bool_parse(value, &suc); + } else { return E_BAD_KEY; } @@ -98,6 +106,9 @@ void UADC_writeIni(Unit *unit, IniWriter *iw) iw_entry_d(iw, "buffer_size", priv->cfg.buffer_size); iw_cmt_newline(iw); + iw_comment(iw, "Enable continuous sampling with averaging"); + iw_comment(iw, "Caution: This can cause DAC output glitches"); + iw_entry_s(iw, "averaging", str_yn(priv->cfg.enable_averaging)); iw_comment(iw, "Exponential averaging coefficient (permil, range 0-1000 ~ 0.000-1.000)"); iw_comment(iw, "- used formula: y[t]=(1-k)*y[t-1]+k*u[t]"); iw_comment(iw, "- not available when a capture is running"); diff --git a/units/adc/unit_adc.c b/units/adc/unit_adc.c index a7d6d0b..e13976b 100644 --- a/units/adc/unit_adc.c +++ b/units/adc/unit_adc.c @@ -191,8 +191,13 @@ static error_t UADC_handleRequest(Unit *unit, TF_ID frame_id, uint8_t command, P return E_BUSY; } + if (! priv->cfg.enable_averaging) { + com_respond_str(MSG_ERROR, frame_id, "Averaging disabled"); + return E_FAILURE; + } + if (priv->real_frequency_int > UADC_MAX_FREQ_FOR_AVERAGING) { - com_respond_str(MSG_ERROR, frame_id, "Too fast for smoothing"); + com_respond_str(MSG_ERROR, frame_id, "Too fast for averaging"); return E_FAILURE; }