Skip to content

PDM library with examples #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Aug 13, 2019
Merged

PDM library with examples #44

merged 16 commits into from
Aug 13, 2019

Conversation

nseidle
Copy link
Member

@nseidle nseidle commented Aug 7, 2019

This adds PDM library support for the mics on the three main carriers (ATP, Nano, and R3) as well as the Edge2.

Library requires user to create an array and pass that array into the .getData() function. This allows the user to locally manipulate the PDM data once it has been obtained from DMA.

Currently the library is only built for single channel mic setups.

I've included a few examples to demo how to get PDM working. The examples are not as simple as I'd like but the FFT function has to be there. Loudest freq works very well! Tested using https://www.szynalski.com/tone-generator/ and holding mic up to speaker.

This PR adds MIC_CLOCK and MIC_DATA to the carrier variant.h files so that the myPDM.begin() can be called without options. However, .begin can be called with Data and Clock pins (not pads) if the user has their own variant without MIC_CLOCK and MIC_DATA defined.

I'm not sure what the policy is on hiding HAL functions from sketches but the examples call am_hal_interrupt_master_disable(), am_hal_sysctrl_sleep(AM_HAL_SYSCTRL_SLEEP_DEEP), etc. We could repackage (redefine) these if we wanted but I'm okay leaving them as they are.

@oclyke
Copy link
Contributor

oclyke commented Aug 8, 2019

Cool! Looks like you solved the HAL exposure with noInterrupts. What will happen if someone calls .begin without any arguments with a variant that does not have MIC_CLOCK and MIC_DATA defined? Maybe consider adding some

#ifndef MIC_CLOCK
#define MIC_CLOCK (some default value here, or a value that could cause a warning in ".begin")
#endif

and similar for the MIC_DATA.

I added a header file just for this called ap3_post_variant.h which is included after the variant to allow the variants definitions to override.

@nseidle
Copy link
Member Author

nseidle commented Aug 13, 2019

I'm hesitant to build in ifndefs. They should be in the variant and if they're not, the compilation should throw some warning or fail. If we quietly assign pins it will likely lead to a lot of lost time by the person writing code .

I can put the following in pdm.h

#ifndef MIC_CLOCK
#warning "Mic CLOCK pin not defined in variant. Using default."
#define MIC_CLOCK 37
#endif

This compiles but currently platform.txt has the -w compiler setting which inhibits all warnings. Removing -w we get the following output which I think is pretty good:

image

Which I think is awesome! Note that this output is limited to just our warning because my sketch is precompiled. If we recompile the entire system we get additional warnings, some of which are really interesting (I need to look into the ap3_analog.cpp warning).

image

Do you know of a way to change compiler flags based on verbose selection? I don't think there's a way. So I'm on the fence as to whether we remove the -w flag from platform.txt. Exposing more scary output to the IDE is poor for a large number of users.

@nseidle
Copy link
Member Author

nseidle commented Aug 13, 2019

This branch now removes -w flag so merge with caution. But it will let you see what the output looks like.

@nseidle
Copy link
Member Author

nseidle commented Aug 13, 2019

Alternatively,

#pragma message ("Mic pin not defined")

Works with -w switch (warnings suppressed):

image

So we could control the output a little more finely.

@oclyke
Copy link
Contributor

oclyke commented Aug 13, 2019

Aha! Nice idea, and also I wonder if that could be why we did not get a warning about the lack of a return value? (However I feel like that should have been an error - anyway...)

I'm fairly sure we can add a menu option that controls this behavior -- however maybe it is not necessary? If a user checks the 'verbose output' option then they likely are either advanced enough to understand or they have been instructed to do so in order to get the maximum amount of debug info for someone else.

For now I will merge this PR because AFAIK all warnings will have been sorted out in the main branch.

@oclyke
Copy link
Contributor

oclyke commented Aug 13, 2019

Ahh.... hmm I actually prefer removing the -w flag -- it allowed me to find another non-return error. But you can still use #pragma for the warning. I also changed the SPI library to operate the way you are describing. (In main branch)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants