Strange impact of CH_DBG options

[Michail]

Hello!

All about Chibios 3.0 from trunk, and STM32F031F4.

I'm using this driver for hymidity sensor DHT11 (founded somewhere on the internet and some modified):

Code: Select all

#include "ch.h"
#include "hal.h"
#include "ext.h"
#include "chconf.h"
#include "core.h"

#if DHT11_PRESENT

#include <DHT11_hal.h>

/*===========================================================================*/
/* Driver local definitions.                                                 */
/*===========================================================================*/

/*===========================================================================*/
/* Driver exported variables.                                                */
/*===========================================================================*/

GPTDriver GPTD17;
dht11_t DHTD1;

#define Timer_Clock 100000

#define T172US(n) ((((n) - 1UL) * (1000000UL / Timer_Clock)) + 1UL)

/*
 * GPT2 configuration.
 */
static const GPTConfig gpt17cfg =
{ Timer_Clock, /* 100kHz timer clock.*/
NULL, /* Timer callback.*/
0, 0 };

/**
 * @brief   Enables the TIM3 peripheral clock.
 * @note    The @p lp parameter is ignored in this family.
 *
 * @param[in] lp        low power enable flag
 *
 * @api
 */
#define rccEnableTIM17(lp) rccEnableAPB2(RCC_APB2ENR_TIM17EN, lp)

/**
 * @brief   Disables the TIM3 peripheral clock.
 * @note    The @p lp parameter is ignored in this family.
 *
 * @param[in] lp        low power enable flag
 *
 * @api
 */
#define rccDisableTIM17(lp) rccDisableAPB2(RCC_APB2ENR_TIM17EN, lp)

/**
 * @brief   Resets the TIM3 peripheral.
 *
 * @api
 */
#define rccResetTIM17() rccResetAPB2(RCC_APB2RSTR_TIM17RST)
/** @} */

/*===========================================================================*/
/* Driver local variables and types.                                         */
/*===========================================================================*/

/*
 * @brief   ...
 */
dht11_t *sensor_handlers[EXT_MAX_CHANNELS];

/*===========================================================================*/
/* Driver local functions.                                                   */
/*===========================================================================*/

/*
 * @brief
 */
inline bool lldLock(lld_lock_t *lock)
{
   bool locked = false;
   //
   chSysLock();
   if (lock->flag == false)
   {
      lock->flag = true;
      locked = true;
   }
   chSysUnlock();
   return locked;
}

/*
 * @brief
 */
inline void lldUnlock(lld_lock_t *lock)
{
   //
   chSysLock();
   if (lock->flag == true)
   {
      lock->flag = false;
   }
   chSysUnlock();
}

/*
 * @brief
 */
inline bool lldLockISR(lld_lock_t *lock)
{
   bool locked = false;
   //
   chSysLockFromISR();
   if (lock->flag == false)
   {
      lock->flag = true;
      locked = true;
   }
   chSysUnlockFromISR();
   return locked;
}

/*
 * @brief
 */
inline void lldUnlockISR(lld_lock_t *lock)
{
   //
   chSysLockFromISR();
   if (lock->flag == true)
   {
      lock->flag = false;
   }
   chSysUnlockFromISR();
}

/*
 * @brief   ...
 * @details ...
 */
static void dht11_lld_ext_handler(EXTDriver *extp, expchannel_t channel)
{
   //
   (void) extp;
   dht11_t *sensor = sensor_handlers[channel];
   if (sensor == NULL)
   {
      return;
   }
   //
   if (lldLockISR(&sensor->lock) == true)
   {
      switch (sensor->state)
      {
      case DHT11_WAIT_RESPONSE:
         sensor->bit_count++;
         if (sensor->bit_count == 3)
         {
            sensor->bit_count = 0;
            sensor->data = 0;
            sensor->state = DHT11_READ_DATA;
         }
         break;
      case DHT11_READ_DATA:
         sensor->bit_count++;
         if (sensor->bit_count % 2 == 1)
         {
            sensor->time_measurment = gptGetCounterX(&GPTD17);
            sensor->data <<= 1;
         }
         else
         {
            volatile systime_t tmp_time = gptGetCounterX(&GPTD17);
            sensor->time_measurment = tmp_time - sensor->time_measurment;
            if (T172US(sensor->time_measurment) > 50)
            {
               sensor->data += 1;
            }
         }
         if (sensor->bit_count == 64)
         {
            sensor->bit_count = 0;
            sensor->crc = 0;
            sensor->state = DHT11_READ_CRC;
         }
         break;
      case DHT11_READ_CRC:
         sensor->bit_count++;
         //sensor->crc = sensor->crc << 1;
         if (sensor->bit_count % 2 == 1)
         {
            sensor->time_measurment = gptGetCounterX(&GPTD17);
            sensor->crc <<= 1;
         }
         else
         {
            volatile systime_t tmp_time = gptGetCounterX(&GPTD17);
            sensor->time_measurment = tmp_time - sensor->time_measurment;
            if (T172US(sensor->time_measurment) > 40)
            {
               sensor->crc += 1;
            }
         }
         if (sensor->bit_count == 16)
         {
            chSysLockFromISR();
            extChannelDisableI(sensor->ext_drv, sensor->ext_pin);
//            if (chVTIsArmedI(&sensor->timer) == true)
//            {
               chVTResetI(&sensor->timer);
//            }
            gptStopTimerI(&GPTD17);
            chSysUnlockFromISR();
            sensor->temp = (sensor->data & 0xFF00) >> 8;
            sensor->humidity = (sensor->data & 0xFF000000) >> 24;
            sensor->state = DHT11_READ_OK;
         }
         break;
      case DHT11_UNINIT:
      case DHT11_IDLE:
      case DHT11_READ_REQUEST:
      case DHT11_READ_OK:
      case DHT11_BUSY:
      case DHT11_ERROR:
         break;
      }
      lldUnlockISR(&sensor->lock);
   }
}

/*
 * @brief   ...
 * @details ...
 */
void dht11_timer_handler(void *p)
{
   dht11_t *sensor = (dht11_t *) p;
   //
   if (lldLockISR(&sensor->lock) == true)
   {
      switch (sensor->state)
      {
      case DHT11_READ_REQUEST:
         chSysLockFromISR();
         chVTSetI(&sensor->timer, MS2ST(5), dht11_timer_handler, sensor);
         extChannelEnableI(sensor->ext_drv, sensor->ext_pin);
         chSysUnlockFromISR();
         palSetPad(sensor->ext_port, sensor->ext_pin);
         palSetPadMode(sensor->ext_port, sensor->ext_pin, PAL_MODE_INPUT_PULLUP);
         sensor->bit_count = 0;
         sensor->state = DHT11_WAIT_RESPONSE;
         break;
      case DHT11_UNINIT:
      case DHT11_IDLE:
      case DHT11_WAIT_RESPONSE:
      case DHT11_READ_DATA:
      case DHT11_READ_CRC:
      case DHT11_READ_OK:
      case DHT11_BUSY:
      case DHT11_ERROR:
         chSysLockFromISR();
         extChannelDisableI(sensor->ext_drv, sensor->ext_pin);
         palSetPadMode(sensor->ext_port, sensor->ext_pin, PAL_MODE_OUTPUT_PUSHPULL);
         palSetPad(sensor->ext_port, sensor->ext_pin);
//         if (chVTIsArmedI(&sensor->timer) == true)
//         {
            chVTResetI(&sensor->timer);
//         }
         gptStopTimerI(&GPTD17);
         chSysUnlockFromISR();
         sensor->state = DHT11_ERROR;
//                SerialConsole::debug("dht11Update timer error\r\n");
         break;
      }
      lldUnlockISR(&sensor->lock);
   }
}

/*===========================================================================*/
/* Driver exported functions.                                                */
/*===========================================================================*/

/*
 * @brief   ...
 */
dht11_state_t dht11Init(dht11_t *sensor)
{
   GPTD17.tim = STM32_TIM17;
   gptObjectInit(&GPTD17);

   rccEnableTIM17(FALSE);
   rccResetTIM17();
//   nvicEnableVector(STM32_TIM14_NUMBER, STM32_GPT_TIM14_IRQ_PRIORITY);
   GPTD17.clock = STM32_TIMCLK1;

   gptStart(&GPTD17, &gpt17cfg);

   dht11_state_t state;
   if (lldLock(&sensor->lock) == true)
   {
      sensor->refresh_time = 0;
      sensor->temp = 0;
      sensor->humidity = 0;
      sensor_handlers[sensor->ext_pin] = sensor;

      palSetPadMode(sensor->ext_port, sensor->ext_pin, PAL_MODE_OUTPUT_PUSHPULL);
      sensor->ext_cfg.mode = EXT_CH_MODE_BOTH_EDGES | sensor->ext_mode;
      sensor->ext_cfg.cb = dht11_lld_ext_handler;
      chSysLock();
      extSetChannelModeI(sensor->ext_drv, sensor->ext_pin, &sensor->ext_cfg);
      extChannelDisableI(sensor->ext_drv, sensor->ext_pin);
      chSysUnlock();

      state = sensor->state = DHT11_IDLE;
      lldUnlock(&sensor->lock);
      return state;
   }
   else
   {
      return DHT11_UNINIT;
   }
}

/*
 * @brief   ...
 */
bool dht11Update(dht11_t *sensor, varg_t unused)
{
   //
   (void) unused;
   dht11_state_t state;

   if (lldLock(&sensor->lock) == true)
   {

         // low pulse
         sensor->bit_count = 0;
         state = sensor->state = DHT11_READ_REQUEST;
         palSetPadMode(sensor->ext_port, sensor->ext_pin, PAL_MODE_OUTPUT_PUSHPULL);
         palClearPad(sensor->ext_port, sensor->ext_pin);

         osalSysLock();

         gptStopTimerI(&GPTD17);
         gptStartContinuousI(&GPTD17, 65000);

         chVTSetI(&sensor->timer, MS2ST(25), dht11_timer_handler, sensor);
         
         osalSysUnlock();

         lldUnlock(&sensor->lock);
         //
         return state;
   }
   else
   {
      return DHT11_BUSY;
   }
}

/*
 * @brief   ...
 */
bool dht11GetTemperature(dht11_t *sensor, int8_t *temp)
{
   //
   *temp = sensor->temp;
   return true;
}

/*
 * @brief   ...
 */
bool dht11GetHumidity(dht11_t *sensor, int8_t *humidity)
{
   //
   *humidity = sensor->humidity;
   return true;
}

#endif

It uses Vitrual Timers, GPT17 fot high precision clocks and EXT interrupts.
A separate thread polls the sensor every 3 seconds. Another thread pools DS18B20 sensor which uses USART.

I've encountered a strange problem when disabling CH_DBG options:

Code: Select all

/**
 * @brief   Debug option, system state check.
 * @details If enabled the correct call protocol for system APIs is checked
 *          at runtime.
 *
 * @note    The default is @p FALSE.
 */
#if CH_DEBUG
#define CH_DBG_SYSTEM_STATE_CHECK           TRUE
#else
#define CH_DBG_SYSTEM_STATE_CHECK           FALSE
#endif

/**
 * @brief   Debug option, parameters checks.
 * @details If enabled then the checks on the API functions input
 *          parameters are activated.
 *
 * @note    The default is @p FALSE.
 */
#if CH_DEBUG
#define CH_DBG_ENABLE_CHECKS                TRUE
#else
#define CH_DBG_ENABLE_CHECKS                FALSE
#endif
/**
 * @brief   Debug option, consistency checks.
 * @details If enabled then all the assertions in the kernel code are
 *          activated. This includes consistency checks inside the kernel,
 *          runtime anomalies and port-defined checks.
 *
 * @note    The default is @p FALSE.
 */
#if CH_DEBUG
#define CH_DBG_ENABLE_ASSERTS               TRUE
#else
#define CH_DBG_ENABLE_ASSERTS               FALSE
#endif

/**
 * @brief   Debug option, trace buffer.
 * @details If enabled then the context switch circular trace buffer is
 *          activated.
 *
 * @note    The default is @p FALSE.
 */
#define CH_DBG_ENABLE_TRACE                 FALSE

/**
 * @brief   Debug option, stack checks.
 * @details If enabled then a runtime stack check is performed.
 *
 * @note    The default is @p FALSE.
 * @note    The stack check is performed in a architecture/port dependent way.
 *          It may not be implemented or some ports.
 * @note    The default failure mode is to halt the system with the global
 *          @p panic_msg variable set to @p NULL.
 */
#if CH_DEBUG
#define CH_DBG_ENABLE_STACK_CHECK           TRUE
#else
#define CH_DBG_ENABLE_STACK_CHECK           FALSE
#endif

/**
 * @brief   Debug option, stacks initialization.
 * @details If enabled then the threads working area is filled with a byte
 *          value when a thread is created. This can be useful for the
 *          runtime measurement of the used stack.
 *
 * @note    The default is @p FALSE.
 */
#if CH_DEBUG
#define CH_DBG_FILL_THREADS                 TRUE
#else
#define CH_DBG_FILL_THREADS                 FALSE
#endif

While they set to TRUE, all works fine:

DHT11 - correct with Debug_rs.jpg (176.75 KiB) Viewed 8819 times

Once I'm disabling this debug options - I'm getting strange aperiodic errors on DHT11. Some times values getting very big, just some smaller than 65535 and 16384. DS18B20 works fine both way.

DHT11 - incorrect with wo Debug_rs.jpg (189.57 KiB) Viewed 8819 times

I don't think that it's a Chibios problem, I suppose that it's from some delays caused by this debug checks, maybe they helps get rid of some jitter...
I racked my brains for a week and can't find the couse. Maybe here someone can help me...

[Giovanni]

The only difference should be in execution times, I don't have elements for a better hypothesis. Do you have all debug options enabled?

Giovanni

[Michail]

Do you have all debug options enabled?

Only shown above. In fact, all that I was changing - CH_DEBUG

Thanks for confirming my hypothesis, tomorrow I'll try to set NanoSleeps in code

[Michail]

Hurrah, at last I won it!

Problem was in:

Code: Select all

         chSysLockFromISR();
         chVTSetI(&sensor->timer, MS2ST(5), dht11_timer_handler, sensor);
         extChannelEnableI(sensor->ext_drv, sensor->ext_pin);
         chSysUnlockFromISR();
         palSetPad(sensor->ext_port, sensor->ext_pin);
         palSetPadMode(sensor->ext_port, sensor->ext_pin, PAL_MODE_INPUT_PULLUP);

Changed to:

Code: Select all

         chSysLockFromISR();
         chVTSetI(&sensor->timer, MS2ST(5), dht11_timer_handler, sensor);
         palSetPad(sensor->ext_port, sensor->ext_pin);
         palSetPadMode(sensor->ext_port, sensor->ext_pin, PAL_MODE_INPUT_PULLUP);
         extChannelEnableI(sensor->ext_drv, sensor->ext_pin);
         chSysUnlockFromISR();

Now everything stable with or without debug!!! Thanks to Giovanni and NanoSleep!!!

[DeusExMachina]

Michail, could you share your code? I aslo need code for DHT11

[Michail]

Of course. It's not fully ready, normally works only with 1 sensor on 1 pin, but it works for me on STM32F0x...

Usage is:

Code: Select all

   DHTD1.ext_pin = DHT11_PIN;
   DHTD1.ext_port = GPIOA;
   DHTD1.ext_drv = &EXTD1;
   DHTD1.ext_mode = EXT_CH_MODE_BOTH_EDGES | EXT_CH_MODE_AUTOSTART | EXT_MODE_GPIOA;
   DHTD1.refresh_period = 1000;
   dht11Init(&DHTD1);

   dht11Update(&DHTD1, NULL);

   while (TRUE)
   {
     dht11Update(&DHTD1, NULL);
     chThdSleepSeconds(1);
     dht11GetHumidity(&DHTD1, &humidity);
     dht11GetTemperature(&DHTD1, &temperature);
        }

[rew]

Michail, would be nice to expand this driver to work for DHT22 too. Your DHT11 has "binary RH data" followed by a byte "decimal RH data". You ignore the decimal one if I read the code correctly. My DHT22 has just a 16-bit field that indicates the RH in tenths of a percent. Similarly, the temperature is a 16-bit number that indicates the temperature in tenths of a degree C.

I haven't had time to test your code yet. I have my "CPU hogging" code working. The advantage of my code is that it works on ALL pins. The disadvantage is that it disables interrupts for about 3ms.

My suggestion would be to ALWAYS report the RH and temp in tenth of a percent/centigrade. Your DHT11 will then always report multiples of ten. And on initialization you can specify if you have a DHT11 or a DHT22.

[Michail]

Michail, would be nice to expand this driver to work for DHT22 too. Your DHT11 has "binary RH data" followed by a byte "decimal RH data". You ignore the decimal one if I read the code correctly. My DHT22 has just a 16-bit field that indicates the RH in tenths of a percent. Similarly, the temperature is a 16-bit number that indicates the temperature in tenths of a degree C.

I haven't had time to test your code yet. I have my "CPU hogging" code working. The advantage of my code is that it works on ALL pins. The disadvantage is that it disables interrupts for about 3ms.

My suggestion would be to ALWAYS report the RH and temp in tenth of a percent/centigrade. Your DHT11 will then always report multiples of ten. And on initialization you can specify if you have a DHT11 or a DHT22.

Hi!
I don't have DHT22, so can't test it. Sounds simple, so you can do it yourself and attach it here. If not, I'll look at it later...
This code can work on any pin too, but work of many sensors simultaneously restricted by reinitialisation of Timer used for high precision time measurement. I don't have any free time now to rework it. Once I do it, i'll post code here.

[rew]

Hmm. It seems I've posted a message for a different forum here by accident. Sorry. Moderators feel free to delete my post mentioning "PPA" above.

Same problem here with "not enough time". I have a working driver right now, so priorities have moved elsewhere. I'll work on it ASAP.

[Michail]

I've updated DHT driver.
- now it supports DHT11, DHT22 or both simultaneously (should be defined in .h file). Not tested on DHT22 but should work;
- supports CRC checking;
- supports as many sensors as you want, on any pin simultaneously;
- supports any free timer (not fully tested but works for all timers on STM32F03xF). One (any, not exactly that you use for DHT) of STM32_GPT_USE_TIMx should be enabled in mcuconf.h to enable ChibiOS GPT driver (I do not know how to work around this without modifing ChibiOS source, so that way now);
- supports asynchronous dht11_state_t dht11StartUpdate(dht11_t *sensor) and synchronous dht11_state_t dht11Update(dht11_t *sensor, uint8_t retry) update with defined number of retries in case of errors.

Usage:
DHT11_hal.h:

Code: Select all

#define DHT_USE_DHT11 TRUE

#define DHT_USE_DHT22 TRUE

#define DHT_USE_CRC_CHECKSUM TRUE

#define DHT_USE_TIMER   17
#define DHT_TIMER_USE_BUS   APB2

your source:

Code: Select all

  dht11_t DHTD1, DHTH2; //yes, now they are not defined in driver
  DHTD1.dht_type = DHT_DHT11;  //should not be defined if one of DHT_USE_DHT11 or DHT_USE_DHT22 is FALSE
  DHTD1.ext_pin = GPIOA_PIN0; //DHT11 data pin
  DHTD1.ext_port = GPIOA; //port
  DHTD1.ext_drv = &EXTD1; 
  DHTD1.ext_mode = EXT_CH_MODE_BOTH_EDGES | EXT_CH_MODE_AUTOSTART | EXT_MODE_GPIOA;
  dht11Init(&DHTD1);


  DHTD2.dht_type = DHT_DHT22;
  DHTH2.ext_pin = GPIOB_PIN3;
  DHTH2.ext_port = GPIOB;
  DHTH2.ext_drv = &EXTD1;
  DHTH2.ext_mode = EXT_CH_MODE_BOTH_EDGES | EXT_CH_MODE_AUTOSTART | EXT_MODE_GPIOB;
  dht11Init(&DHTH2);

  while (TRUE) {
        uint32_t error = 0;
        int16_t humidity, temperature;

/*
 * Synchronous update for DHTD1
 */
        if (dht11Update(&DHTD1, 3) != DHT11_READ_OK) {
          error++;
        }
        else {
          error = 0;
          dht11GetHumidity(&DHTD1, &humidity);
          dht11GetTemperature(&DHTD1, &temperature);
        }

/*
 * Asynchronous update for DHTD2
 */
        dht11StartUpdate(&DHTD2);
        osalThreadSleepMilliseconds(100);
        if (DHTD2.state != DHT11_READ_OK) {
          error++;
        }
        else {
          error = 0;
          dht11GetHumidity(&DHTD2, &humidity);
          dht11GetTemperature(&DHTD2, &temperature);
        }
        ...
  }

For me it works like a charm on STM32F030F, STM32F031F in any combination of pins and timer. For STM32F100C it compiles fine and I think should work, but not really tested yet, I'll try it on weekends if I'll have free time.

PS: I think last 5-6 posts can be freely moved to other topic. Sorry for offtopic