MakeCode issue with MakeCode sonar extension

microbit-carlos commented 1 month ago

Issue:

https://github.com/microsoft/pxt-microbit/issues/5327

Extension:

https://github.com/microsoft/pxt-sonar/blob/master/main.ts

MakeCode pulseIn:

https://github.com/microsoft/pxt-microbit/blob/0f801980bb1efb6755f305783a4275417dc604ae/libs/core/pins.cpp#L294

CODAL getPulseUs:

https://github.com/lancaster-university/codal-nrf52/blob/8802eb49140e0389e535cb6160d9080efd951ba7/source/NRF52Pin.cpp#L927

first step would be to write the SR04 code in C++ and see if we can replicate the issue where only 0-10cm is measured.

martinwork commented 1 month ago

This is a C++ version of MakeCode code, except I separated CODAL and DAL pulseIn so I could choose either.

The pins (13, 14) are selected at the top of forever(). The distances are sent to DMESG.

The forever loop includes calls to reset both pins to digital inputs with pull down, immediately after receiving the pulse, and before the recommended 60ms pause between readings, which seems to give very good results.

Without resetting the pins, most values after the first are low and variable.

Example hex files: sonar-v2.zip

#include "MicroBit.h"

#ifndef MICROBIT_CODAL
  #ifdef CODAL_CONFIG_H
    #define MICROBIT_CODAL 1
  #else
    #define MICROBIT_CODAL 0
  #endif
#endif

#if MICROBIT_CODAL
#else
#define target_wait_us(us) wait_us(us)
#endif

void  sonarTrigger( MicroBitPin *trig);
int   pulseInCODAL( MicroBitPin *pin, bool high, int maxDuration = 2000000);
int   pulseInDAL( MicroBitPin *pin, bool high, int maxDuration = 2000000);
int   pulseIn( MicroBitPin *pin, bool high, int maxDuration = 2000000);
void  resetPin( MicroBitPin *pin);
void  forever();

MicroBit uBit;

int main()
{
  uBit.init();
  uBit.serial.setTxBufferSize(254);
  create_fiber( forever);
  release_fiber();
  return 0;
}

void forever()
{
    MicroBitPin *pinTrig = &uBit.io.P13;
    MicroBitPin *pinEcho = &uBit.io.P14;

    while (true)
    {
      DMESG( "");

      sonarTrigger( pinTrig);

      int cm = pulseInCODAL( pinEcho, true, 500 * 58) / 58;
      //int cm = pulseInDAL( pinEcho, true, 500 * 58) / 58;
      DMESG( "cm:%d", cm);

      resetPin( pinEcho);
      resetPin( pinTrig);

      uBit.sleep( 60);
    }
}

void sonarTrigger( MicroBitPin *trig)
{
  //DMESG("sonarTrigger(%p)", trig);
#if MICROBIT_CODAL
  trig->setPull( PullMode::None);
#else
  trig->setPull( PullNone);
#endif
  trig->setDigitalValue(0);
  target_wait_us(2);
  trig->setDigitalValue(1);
  target_wait_us(10);
  trig->setDigitalValue(0);
}

int pulseInCODAL( MicroBitPin *pin, bool high, int maxDuration) {
#if MICROBIT_CODAL
  //DMESG("pulseInCODAL(%p)", pin);
  pin->setPolarity(high ? 1 : 0);
  int period = pin->getPulseUs(maxDuration);
  // timeout
  if (DEVICE_CANCELLED == period)
      return 0;
  // success!
  return period;
#else
  return 0;
#endif
}

int pulseInDAL( MicroBitPin *pin, bool high, int maxDuration) {
  //DMESG("pulseInDAL(%p)", pin);
  int pulse = high ? 1 : 0;
  uint64_t tick =  system_timer_current_time_us();
  uint64_t maxd = (uint64_t)maxDuration;
  while(pin->getDigitalValue() != pulse)
  {
      if(system_timer_current_time_us() - tick > maxd)
          return 0;
  }

  uint64_t start =  system_timer_current_time_us();
  while(pin->getDigitalValue() == pulse)
  {
      if(system_timer_current_time_us() - tick > maxd)
          return 0;
  }
  uint64_t end =  system_timer_current_time_us();
  return end - start;
}

int pulseIn( MicroBitPin *pin, bool high, int maxDuration) {
#if MICROBIT_CODAL
  return pulseInCODAL( pin, high, maxDuration);
#else
  return pulseInDAL( pin, high, maxDuration);
#endif
}

void resetPin( MicroBitPin *pin)
{
  int scratch;
//  scratch = pin->getAnalogValue();
  scratch = pin->getDigitalValue();
#if MICROBIT_CODAL
  pin->setPull( PullMode::Down);
#else
  pin->setPull( PullDown);
#endif
}

martinwork commented 1 month ago

Even without the resetPin calls in the forever loop above, CODAL currently resets the echo pin and creates a new PulseIn for each call to getPulseUs (see why below).

I modified getPulseUs to avoid the reset. I'm not sure if it helped, but it was not nearly as good as "manually" resetting the pins before the pause between readings.

I don't know why it's different. Could it be better for the sensor?

Details of why the reset occurs...

https://github.com/lancaster-university/codal-nrf52/blob/8802eb49140e0389e535cb6160d9080efd951ba7/source/NRF52Pin.cpp#L927

There's a call to getDigitalValue to ensure the pin is in digital input mode.

But since MakeCode v5, CODAL v0.2.40 the test at the top of getDigitalValue has been changed to check for a locked peripheral obj. The PulseIn is an obj but it's not locked, so the call disconnects the PulseIn and resets the interrupt behaviour, which is noticed on the next line in getPulseUs.

Here's the change at line 335 https://github.com/lancaster-university/codal-nrf52/commit/24ea96296b341b5dc4c83655670d3526cd1c44ab

Note that the code at line 338 has gone, although the comment about fast switching remains.

https://github.com/lancaster-university/codal-nrf52/blob/8802eb49140e0389e535cb6160d9080efd951ba7/source/NRF52Pin.cpp#L274

martinwork commented 1 month ago

So fast input/output switching support has been lost (for one wire sensors, or sonar sensors that use a single pin?) https://github.com/lancaster-university/codal-nrf52/commit/f050e2ec3500cc39ec03e3a0c0dcf065c8ca4ac2

lancaster-university / codal-microbit-v2

MakeCode issue with MakeCode sonar extension #439