-
Notifications
You must be signed in to change notification settings - Fork 3
libraries/PDM/src/PDM.cpp: Add PDM begin and end. #160
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
base: pdm_integration
Are you sure you want to change the base?
Conversation
Signed-off-by: Rathan25 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please complete the work or open against a feature branch.
Also, make sure the checks are passing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start 👍
We have to also start thinking of the tests in arduino-core-test
In micropython we have faked a pcm signal with gpio toggling and then read the pattern from the receiver:
https://github.com/Infineon/micropython/blob/ports-psoc6-main/tests/ports/psoc6/board_ext_hw/multi/pdm_pcm_rx.py
https://github.com/Infineon/micropython/blob/ports-psoc6-main/tests/ports/psoc6/board_ext_hw/multi/pdm_pcm_tx.py
# Instances (KEYWORD2) | ||
####################################### | ||
|
||
PDM KEYWORD2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we are going to be able to create multiple instances, this should be a data type -> KEYWORD1
#include "Arduino.h" | ||
#include "cyhal_pdmpcm.h" | ||
|
||
#define PDM_DATA P10_5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should create a constructor which accepts the pins, like ->
https://github.com/Infineon/arduino-core-psoc6/blob/main/libraries/Wire/src/Wire.h#L17*
Then we can have a pre-instantiated object like this:
https://github.com/Infineon/arduino-core-psoc6/blob/main/libraries/Wire/src/Wire.cpp#L236-L241
And those defines are part of the pins_arduino.h -> https://github.com/Infineon/arduino-core-psoc6/blob/main/libraries/Wire/src/Wire.cpp#L236-L241
return; \ | ||
} | ||
|
||
bool PdmClass::begin(int channels, int sample_rate) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, there are few more things that need to be taken care in the begin:
- irq setup
- dma
- ringbuffer usage (there is a coreAPI ringbuffer that maybe can be used -> https://github.com/arduino/ArduinoCore-API/blob/master/api/RingBuffer.h
- The clock also needs to be setup depending on the sampling rate.
https://github.com/Infineon/micropython/blob/ports-psoc6-main/ports/psoc6/machine_pdm_pcm.c#L435-L499
It is fine, you can gradually add that in subsequent PR.
#define PDM_DATA P10_5 | ||
#define PDM_CLK P10_4 | ||
|
||
#define SAMPLE_RATE_HZ 8000u |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These defines can be private, in the .cpp?
#define SAMPLE_RATE_HZ 8000u | ||
#define DECIMATION_RATE 64u | ||
|
||
class PdmClass { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would name it PDMClass
, as here -> https://github.com/arduino/ArduinoCore-mbed/blob/main/libraries/PDM/src/PDM.h
PR Description.
Done unit test with channels and passing sample rate. For single channel it configures with left channel . Going forward need to check with application
Captured logs:
