lancaster-university / codal-samd21

MIT License
3 stars 1 forks source link

Revision of PDM code to have proper sinc filter and maybe dB-gain-managed output #2

Open ladyada opened 6 years ago

ladyada commented 6 years ago

Dean @deanm1278 kindly volunteered to port his Arduino-calibrated code to CODAL!

pelikhan commented 6 years ago

Thank you @deanm1278!

jamesadevine commented 6 years ago

Fantastic news @ladyada! Thanks @deanm1278 :smile:

deanm1278 commented 6 years ago

hi @jamesadevine I have a Q: What am I doing wrong in this code:


#include "CircuitPlayground.h"
#include "SAMD21DMAC.h"
#include "SAMD21PDM.h"
#include "LevelDetector.h"

CircuitPlayground cplay;

SAMD21DMAC dma;
SAMD21PDM mic(cplay.io.microphoneData, cplay.io.microphoneClock, dma, 22000);
LevelDetector lvl(mic, 0, 0);

void test(Event){
    asm volatile("NOP;");
}

int main(){
    cplay.messageBus.listen(DEVICE_ID_SYSTEM_LEVEL_DETECTOR,LEVEL_THRESHOLD_HIGH,test); 

    mic.enable();   

    release_fiber();
}

This should initialize the mic and then generate an event when the PDM level goes above or below 0 right?

LevelDetector::pullRequest() only gets called once and then DataStream seems to spin out here on all further requests:

if (full() && this->isBlocking == false)
        return DEVICE_NO_RESOURCES;

Is there something else I should be doing to release the buffer?

jamesadevine commented 6 years ago
LevelDetector lvl(mic, 0, 0);

Off the top of my head, you may be hitting a corner case in the code, I think it presumes that the two values are different, not the same...

LevelDetector lvl(mic, 1, 0);

Would likely work, I'll give it a look in a bit :smile:

jamesadevine commented 6 years ago

@deanm1278 HEY!

Sorry this took a little while to get back to you, here's a sample that will work...:

#include "CircuitPlayground.h"
#include "SAMD21DMAC.h"
#include "SAMD21PDM.h"
#include "LevelDetector.h"
#include "StreamNormalizer.h"

CircuitPlayground cplay;

SAMD21DMAC dma;
SAMD21PDM mic(cplay.io.microphoneData, cplay.io.microphoneClock, dma, 22000);
LevelDetector lvl(mic.output, 70, 30);

void high(Event)
{
    cplay.io.led.setDigitalValue(1);
}

void low(Event)
{
    cplay.io.led.setDigitalValue(0);
}

int main()
{
    cplay.messageBus.listen(DEVICE_ID_SYSTEM_LEVEL_DETECTOR,LEVEL_THRESHOLD_HIGH,high);
    cplay.messageBus.listen(DEVICE_ID_SYSTEM_LEVEL_DETECTOR,LEVEL_THRESHOLD_LOW,low);

    mic.enable();

    release_fiber();
}

I've also added some samples we had knocking about on an old repository to the circuit-playground repo (https://github.com/lancaster-university/codal-circuit-playground/tree/master/samples)

jamesadevine commented 6 years ago

P.S. I've found that sample works best if you blow directly at the mic :smile:

deanm1278 commented 6 years ago

@jamesadevine ok thanks very much! I'll check it out when I have a minute to get back to this. Was mine not working because I'm supposed to use mic.output instead of mic?

jamesadevine commented 6 years ago

Yes, that's correct :smile: sorry didn't spot it in my cursory glance...

deanm1278 commented 6 years ago

ok @jamesadevine @pelikhan I have made a bunch of PDM updates here: https://github.com/adafruit/codal-samd21/blob/master/source/SAMD21PDM.cpp

and have also updated the LevelShifter stream to use dB SPL here: https://github.com/adafruit/codal-core/commit/a4e9a3679b298f1832499581d5987944a5607224

If this is not the correct place for the dB SPL code, or it should go in it's own stream please let me know.

So that I can be sure things are correct before I submit a PR, is there any way to print things via USB CDC similar to Arduino Serial.print()?

Thanks!

jamesadevine commented 6 years ago

There's a uf2 equivalent yes, see uf2tool.

Alternately just attach a usb to serial cable, to TX and RX on the cplay.

By default dmesg uses uf2 to debug, if you flip the DMESG_SERIAL_DEBUG flag and have DMESG_BUFFER_SIZE set to something reasonable, it will use Serial instead.

jamesadevine commented 6 years ago

@deanm1278 WRT your code, I think there should be a separation of concerns... It seems like your modifications to LevelDetector has two purposes:

  1. Conversion to DB.
  2. Floating point based threshold detection.

So for me it feels like these should be two separate nodes in the stream:

Microphone -> decibels conversion -> floating point level detector

I think it's useful to have both versions of the level detector, so I think that your floating point level detector should be a separate class.

What do you think @deanm1278 @finneyj ? Does it make sense to separate out the decibels conversion?

deanm1278 commented 6 years ago

Ok I've broken out a separate dB SPL level detector (see my PRs in codal-samd21 and codal-core). IMO a separate db conversion stream would be kind of redundant since it's only meaningful when calculated over a given window. LevelDetectorSPL.getValue() can be used to return the value of the last calculation if no actual level detection is needed.

jamesadevine commented 6 years ago

@deanm1278 Agreed, cool. I've given your PRs a quick review, it took me a little while to be able to test :smile:

I like that we now use an SI unit for the microphone, it opens up a whole bunch more educational scenarios in the classroom. Thanks a lot!