lancaster-university / codal-microbit-v2

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

`MIC_ENABLE` macro is currently not doing anything #223

Open microbit-carlos opened 2 years ago

microbit-carlos commented 2 years ago

https://github.com/lancaster-university/codal-microbit-v2/blob/864e7b87d2968c91c84e8eb204836b67accabda4/model/MicroBit.h#L327

It's not really used in the CODAL codebase, but it could stop other projects from using the macro, like in MakeCode: https://github.com/microsoft/pxt-microbit/blob/2615cbb0d5064d4235cd6c031cb268e2a75191cc/libs/microphone/microphonehw.cpp#L36-L38

#ifndef MIC_ENABLE
#define MIC_ENABLE microphone.enable()
#endif

And I wouldn't be surprised if that broke this test file: https://github.com/lancaster-university/microbit-v2-samples/blob/6bdbdbeffee3146a1c05e5069ba5605f873a7002/source/samples/MicrophoneTest.cpp#L134

MakeCodeMicrophoneTemplate() MIC_INIT { MIC_ENABLE; }

So we should either fix the macro or remove it.

microbit-carlos commented 2 years ago

Is actually used in MakeCode: https://github.com/microsoft/pxt-microbit/blob/d80514bda223f729d452c1e51047f3272a2317fb/libs/microphone/microphonehw.cpp#L46

class WMicrophone {
  public:
    MIC_DEVICE microphone;
    LevelDetectorSPL level;
    WMicrophone() MIC_INIT { MIC_ENABLE; }
};

MIC_INIT should be expanding to microphone(uBit.audio.mic) , level(* uBit.audio.levelSPL) https://github.com/lancaster-university/codal-microbit-v2/blob/864e7b87d2968c91c84e8eb204836b67accabda4/model/MicroBit.h#L323-L325

So I guess the microphone in CODAL is already enabled by default and WMicrophone constructor doesn't need any additional code to be run? In that case it might be better to remove the commented out code from the macro with a comment or something explaining it.

microbit-carlos commented 1 year ago

Let's use this issue to track syncing with the MakeCode team to remove their own mic initialisation. Then we can remove this macro from the micro:bit codal codebase as well.