search

lancaster-university / codal-microbit-v2

CODAL target for the micro:bit v2.x series of devices
MIT License
41 stars 50 forks source link

Driving the speaker pin with hardware PWM produces only noise the first time #313

Open microbit-carlos opened 1 year ago

microbit-carlos commented 1 year ago

This example programme produces a weird noise the first loop iteration, and the correct tone the others. MICROBIT.hex.zip

#include "MicroBit.h"

MicroBit uBit;

static void playTone(int frequency, int ms) {
    Pin *audioPin = &uBit.io.speaker;
    if (frequency) {
        audioPin->setAnalogPeriodUs(1000000 / frequency);
        audioPin->setAnalogValue(127);
    }
    uBit.sleep(ms);
    audioPin->setAnalogValue(0);
}

int main() {
    uBit.init();

    // Don't know if we are meant to disable the pipeline to drive the pin directly, but this doesn't make a difference
    uBit.audio.disable();   

    for (int i = 0; i < 4; i++) {
        playTone(440, 500);
        uBit.sleep(500);
    }
}
microbit-carlos commented 1 year ago

This is still replicable in v0.2.55.

microbit-carlos commented 11 months ago

I can confirm this is still replicable with CODAL v0.2.57

martinwork commented 11 months ago

The first call to setAnalogPeriodUs does nothing because the pin hasn't been configured as an analogue output. https://github.com/martinwork/codal-nrf52/blob/master/source/NRF52Pin.cpp#L633

microbit-carlos commented 11 months ago

Thanks Martin! I can confirm that switch the order and executing first setAnalogValue() and then setAnalogPeriodUs() fixes the issue.

However, it makes sense for the user to want to set up the analog out configuration before changing the pin mode to start outputting the signal.

martinwork commented 11 months ago

Yes, I don't know why it doesn't call setAnalogValue(0) if the pin isn't already configured, but it might be awkward to remember the setting until setAnalogValue is called. https://github.com/lancaster-university/codal-nrf52/blob/master/source/NRF52Pin.cpp#L631

The DAL would have to change too. https://github.com/lancaster-university/microbit-dal/blob/js-event-semantics/source/drivers/MicroBitPin.cpp#L417

It can't find an explicit mention in the CODAL source or Python docs, but it's there in DAL and MakeCode. https://github.com/lancaster-university/microbit-dal/blob/js-event-semantics/source/drivers/MicroBitPin.cpp#L413 https://lancaster-university.github.io/microbit-docs/ubit/io/#setanalogperiodus https://makecode.microbit.org/reference/pins/analog-set-period https://microbit-micropython.readthedocs.io/en/v2-docs/pin.html?highlight=write_analog#microbit.MicroBitDigitalPin.set_analog_period

microbit-carlos commented 11 months ago

Discussed with @finneyj and we should be able to configure analog settings before using the pin as an analog input/output, so we can consider this a bug to fix.

With that in mind, this MicroPython issue is also related: