Skip to content

Conversation

aykevl
Copy link
Member

@aykevl aykevl commented May 10, 2020

This PR implements pin interrupts for Nordic devices and for SAM D21 devices.

I have kept the API very simple, so it is easy to generalize to many different microcontrollers. It is also possible to make it faster, but that would make the API also less flexible. Right now it will just read the callback from a global, which should be good enough for most purposes.

Something else that it doesn't implement (yet) is level interrupts (as opposed to edge interrupts). Level interrupts, while (probably) easier to implement in hardware, are harder to use in practice. The main advantage of level interrupts appears to be that some chips support them even in very low power states. For simplicity, I've just left them out in this version.

I have also implemented interrupts for AVR devices (Arduino Uno and Digispark), so that means 3 different chip families support this API. However, the code needs to be cleaned up a bit and I would like to clean up pin handling first (see #1093).

There are some features that I have not implemented, such as using one interrupt channel for multiple pins on the SAM D21. This would have complicated things so again, I've just left that out.

@aykevl aykevl requested a review from deadprogram May 10, 2020 18:25
@aykevl aykevl force-pushed the interrupts-pins branch from 06d0bdb to cb03e09 Compare May 10, 2020 18:27
@deadprogram
Copy link
Member

@aykevl merge conflict needs resolution, please.

@sago35
Copy link
Member

sago35 commented May 12, 2020

@aykevl
I've started making an atsamd51 version
I still need to clean up the code, but it's working.
I'll push it later.

Here's something I noticed about SAMD21, so I'll report it.

  1. Parentheses are required.
  • Addr.Set((addr.Get() &^ 0xf << pos) | uint32(change)<<pos)
  • addr.Set((addr.Get() &^ (0xf << pos)) | uint32(change)<<pos)
  1. You need to OR multiple EICs to enable them.
  • sam.EIC.INTENSET.Set(1 << extint)
  • sam.EIC.INTENSET.SetBits(1 << extint)

Translated with www.DeepL.com/Translator (free version)

@aykevl aykevl force-pushed the interrupts-pins branch 2 times, most recently from 435688c to 7d94489 Compare May 12, 2020 13:28
@aykevl
Copy link
Member Author

aykevl commented May 12, 2020

@deadprogram rebased.

  1. Parentheses are required.
  • Addr.Set((addr.Get() &^ 0xf << pos) | uint32(change)<<pos)
  • addr.Set((addr.Get() &^ (0xf << pos)) | uint32(change)<<pos)

Oops, thank you for catching that! I've updated the PR.

  1. You need to OR multiple EICs to enable them.
  • sam.EIC.INTENSET.Set(1 << extint)
  • sam.EIC.INTENSET.SetBits(1 << extint)

I don't think so. INTENSET already performs the SetBits operation in hardware. The manual says:

Writing a zero to this bit has no effect.
Writing a one to this bit will set the External Interrupt x Enable bit, which enables the external interrupt.

@sago35
Copy link
Member

sago35 commented May 14, 2020

  1. You need to OR multiple EICs to enable them.
  • sam.EIC.INTENSET.Set(1 << extint)
  • sam.EIC.INTENSET.SetBits(1 << extint)

I don't think so. INTENSET already performs the SetBits operation in hardware. The manual says:

Writing a zero to this bit has no effect.
Writing a one to this bit will set the External Interrupt x Enable bit, which enables the external interrupt.

I made a mistake.
It certainly says so in the manual.

@sago35
Copy link
Member

sago35 commented May 14, 2020

I wrote the code for SAMD51J19A.
I checked with feather-m4 and it seems to be working fine.
I haven't confirmed this with the SAMD51J20A yet.

https://github.com/sago35/tinygo/tree/interrupts-pins-samd51

I'm struggling with how to write the following parts.
Is there a better way to do it?

  1. sam.EIC.CTRLA.Set(0) or sam.EIC.CTRLA.ClearBits(0x02)
  2. Is it better to define it outside of SetInterrupt()?
  3. I'd like to write a little nicer, but interrupt.New() only accepts const values, so this is how it's implemented.
  4. I want to make an example, but the feather-m4 I have doesn't have a button.
    Creating an example for pybadge is one thing, but since there is no machine.Button, the other examples need to be modified.

@aykevl
Copy link
Member Author

aykevl commented May 15, 2020

@sago35 it would be easier to give feedback on an actual (work-in-progress) PR. Can you perhaps make one?

@sago35
Copy link
Member

sago35 commented May 15, 2020

@sago35 it would be easier to give feedback on an actual (work-in-progress) PR. Can you perhaps make one?

I made #1111.

@deadprogram
Copy link
Member

Excellent progress on this! Now merging.

@deadprogram deadprogram merged commit c72f9eb into dev May 28, 2020
@deadprogram deadprogram deleted the interrupts-pins branch May 28, 2020 09:20
@niaow niaow added this to the v0.14 milestone Jun 27, 2020
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