Skip to content
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

PDM_PCM-read, irq and set gain function implementation #187

Open
wants to merge 4 commits into
base: machine-pdm
Choose a base branch
from

Conversation

NikhitaR-IFX
Copy link
Member

Summary

In this PR, all the below functions are implemented but yet to be fully tested.

  • PDM_PCM.irq()
  • PDM_PCM.readinto()
  • PDM_PCM.set_gain()

In the source file, you will find 4 sections:

  1. PDM_PCM higher level MPY functions --> at some point this should move to extmod/ and the functions in this are port independent
  2. Port-specific private functions for DMA and PDM_PCM support
  3. MPY bindings for the module
  4. Implementation to support stream protocol

Test Status:

All the testin so far is being done manually and is later when successful to be added to HIL.

  • PDM_PCM.set_gain() atleast does the job of setting the values and this is ensured as cyhal api is supposed to throw an error if we try to set unacceptable values.
  • PDM_PCM.irq() sets the callback fine.
  • PDM_PCM.readinto() is waiting for the interrupt trigger forever. I suspect the buffer configurations.

Note: Based on further testing, I feel changes would happen only to following functions and you may ignore if needed while reviewing the same:

  • fill_appbuf_from_ringbuf()
  • fill_appbuf_from_ringbuf_non_blocking()

Also I am aware there are printf()/mp_printf() statements in the beginning of every function, but only to help with debugging. This will be replaced later by mplogger_print().

Copy link
Member

@jaenrig-ifx jaenrig-ifx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good, with the structure and the API :)

Only the comments on the documentation. Which is generated here:
https://ifx-micropython.readthedocs.io/en/pdm-read-blocking/psoc6/quickref.html#pdm-pcm

Without tests passing, I will postpone a deeper of the code functionality.

@@ -633,19 +633,31 @@ Constructor
Methods
-------
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The headlines need to have 1 level lower (Methods, Constructor, etc.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And to keep consistency with the rest of the classes, I would name this "PDM-PDM bus"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted

Copy link

@IFX-Anusha IFX-Anusha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good!

Copy link

@ramya-subramanyam ramya-subramanyam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

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.

4 participants