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

Remove unnecessary reference to CircuitPython binding #44

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mattytrentini
Copy link

The include to shared-bindings/microcontroller/__init__.h appears to be unused but makes it difficult to build without CircuitPython.

To be certain, I removed the include and tested building the CircuitPython atmel-samd boards feather_m4_express and trinket_m0 - there were no errors.

Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Thanks!

common_hal_mcu_disable_interrupts() and common_hal_mcu_enable_interupts() are called in samd/samd21/dma.c and samd/external_interrupts.c.

This is noted in #28, with a possible solution. Another refactoring would be to move the body of those routines back to here as new routines, and call them from common_hal/microcontroller/__init__.c.

@dhalbert
Copy link
Contributor

dhalbert commented Jan 8, 2023

The trinket_m0 build is quite minimal, so you may not be hitting those dependencies. The metro_m0_express build is more likely to.

@mattytrentini
Copy link
Author

The trinket_m0 build is quite minimal, so you may not be hitting those dependencies. The metro_m0_express build is more likely to.

Thanks; just tried metro_m0_express; it also builds fine.

@mattytrentini
Copy link
Author

Just for background: I'm adding cap touch support to MicroPython, particularly SAMD. I used the Adafruit_Freetouch library to add the majority of the functionality (for SAMD21 at least, 51 is going to take further effort). However, looking at the TouchIn implementation, samd-peripherals was required - just to configure a GCLK appropriately.

Even that needs a little work since - if I understand correctly, it currently only works for 48MHz

@mattytrentini
Copy link
Author

Apologies, that was a long-winded way of saying that I'm currently focussed on getting cap touch working for MicroPython and this change is all that's required for that. Once I sort out touch I'll come back and take a look at dma.c.

@dhalbert
Copy link
Contributor

dhalbert commented Jan 9, 2023

It turns out this only works because the interrupt routines are also declared here:

https://github.com/adafruit/circuitpython/blob/037bfb39e37af04b74a6f77d8a5eb0eb8cbce271/py/circuitpy_mpconfig.h#L51-L56:

// These critical-section macros are used only a few places in MicroPython, but
// we need to provide actual implementations.
extern void common_hal_mcu_disable_interrupts(void);
extern void common_hal_mcu_enable_interrupts(void);
#define MICROPY_BEGIN_ATOMIC_SECTION() (common_hal_mcu_disable_interrupts(), 0)
#define MICROPY_END_ATOMIC_SECTION(state) ((void)state, common_hal_mcu_enable_interrupts())

Ideally we would like to remove both MicroPython and CircuitPython dependencies from this library.

Aside re SAMD51 touch: we tried reverse-engineering it and failed, and instead reused the implementation we originally developed for nRF, which requires a 1M pull-down. It works quite well.
https://github.com/adafruit/circuitpython/blob/037bfb39e37af04b74a6f77d8a5eb0eb8cbce271/shared-module/touchio/TouchIn.c

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