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.

Sign up for GitHub

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

Working with ATtinyx61 #43

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Working with ATtinyx61 #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b93af7e
Working with ATtinyx61
tredpath Jun 26, 2021
74e0fdd
Update option name
tredpath Jul 9, 2021
94f7738
Check if PORT1 is to be used
tredpath Jul 9, 2021
3c2bcc9
Conditionally allow PORT1 to be disabled
tredpath Jul 9, 2021
0337774
Add disable for x61
tredpath Jul 9, 2021
2c7a9fc
Remove initPinChangeInterrupt
tredpath Jul 9, 2021
6b5d48e
Use true instead of 1
tredpath Jul 9, 2021
d129ff5
Remove initPinChangeInterrupt keyword
tredpath Jul 9, 2021
7ef2dbe
Rearrange disable helper for x61
tredpath Jul 12, 2021
4a28a87
getArrayPosPCINT returning incorrect value
tredpath Jul 12, 2021
4877616
Simplify disable logic
tredpath Jul 12, 2021
13219eb
Always clear interrupt masks on enable
tredpath Jul 12, 2021
e8bf710
Merge branch 'master' into working/attinyx61
tredpath Jul 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 62 additions & 1 deletion src/PinChangeInterrupt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,29 @@ void enablePinChangeInterruptHelper(const uint8_t pcintPort, const uint8_t pcint
#endif
}

// Special case for Attiny x61 where PCMSK0 and PCMSK1 registers
// have initial values of 1 for some reason.
// See datasheet section 9.3.4 and 9.3.5
// https://ww1.microchip.com/downloads/en/devicedoc/atmel-2588-8-bit-avr-microcontrollers-tinyavr-attiny261-attiny461-attiny861_datasheet.pdf
#if defined(GIMSK) && (defined(__AVR_ATtiny261__) || defined(__AVR_ATtiny461__) || defined(__AVR_ATtiny861__))
#if (PCINT_USE_PORT1 == true)
// PCIE0 case
if (!(GIMSK & (1 << PCIE0))) {
// Clear PCINT11:8
PCMSK1 &= ~0x0F;
}
// PCIE1 case
#endif
if (!(GIMSK & (1 << PCIE1))) {
#if (PCINT_USE_PORT1 == true)
// Clear PCINT15:12
PCMSK1 &= ~0xF0;
#endif
// Clear PCINT7:0
PCMSK0 = 0x00;
}
#endif

// Pin change mask registers decide which pins are ENABLE as triggers
#ifdef PCMSK0
#ifdef PCMSK1
Expand Down Expand Up @@ -165,6 +188,21 @@ void enablePinChangeInterruptHelper(const uint8_t pcintPort, const uint8_t pcint
PCICR |= (1 << (pcintPort + PCIE0));
#elif defined(GICR) /* e.g. ATmega162 */
GICR |= (1 << (pcintPort + PCIE0));
#elif defined(GIMSK) && (defined(__AVR_ATtiny261__) || defined(__AVR_ATtiny461__) || defined(__AVR_ATtiny861__))
// This is a special case for Attiny x61 series which was very weird PCINT mapping.
// See datasheet section 9.3.2: http://ww1.microchip.com/downloads/en/devicedoc/atmel-2588-8-bit-avr-microcontrollers-tinyavr-attiny261-attiny461-attiny861_datasheet.pdf
#if (PCINT_USE_PORT1 == true)
if (pcintPort == 1 && pcintMask < (1 << 4)) {
// PCINT11:8 will be enabled with PCIE0
GIMSK |= (1 << PCIE0);
}
else {
#endif
// PCINT7:0 and PCINT15:12 will be enabled with PCIE1
GIMSK |= (1 << PCIE1);
#if (PCINT_USE_PORT1 == true)
}
#endif
#elif defined(GIMSK) && defined(PCIE0) /* e.g. ATtiny X4 */
GIMSK |= (1 << (pcintPort + PCIE0));
#elif defined(GIMSK) && defined(PCIE) /* e.g. ATtiny X5 */
Expand Down Expand Up @@ -247,9 +285,31 @@ void disablePinChangeInterruptHelper(const uint8_t pcintPort, const uint8_t pcin
// if that's the last one, disable the interrupt.
if (*(&PCMSK + pcintPort) == 0) {
disable = true;
}
Copy link
Owner

Choose a reason for hiding this comment

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

That bracket is missing. I guess the code will not compile now.

#endif // ifdef PCMSK0

// This is a special case for Attiny x61 series which was very weird PCINT mapping.
// See datasheet section 9.3.2:
// http://ww1.microchip.com/downloads/en/devicedoc/atmel-2588-8-bit-avr-microcontrollers-tinyavr-attiny261-attiny461-attiny861_datasheet.pdf
#if defined(GIMSK) && (defined(__AVR_ATtiny261__) || defined(__AVR_ATtiny461__) || defined(__AVR_ATtiny861__))
#if (PCINT_USE_PORT1 == true)
if (pcintPort == 1 && pcintMask < (1 << 4)) {
//PCINT11:8 will be disabled with PCIE0
if (!(PCMSK1 & 0x0F)) {
GIMSK &= ~(1 << PCIE0);
}
}
else {
//PCINT7:0 and PCINT15:12 will be disabled with PCIE1
if (!PCMSK0 && !(PCMSK1 & 0xF0)) {
#else
if (!PCMSK0) {
#endif
GIMSK &= ~(1 << PCIE1);
}
#if (PCINT_USE_PORT1 == true)
}
#endif
#else
Copy link
Owner

Choose a reason for hiding this comment

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

We do not need this else path, if you add the return statement, that I've added in my original code snipped. That would be much clearer to separate this special use case.

if(disable)
{
#ifdef PCICR
Expand All @@ -264,6 +324,7 @@ void disablePinChangeInterruptHelper(const uint8_t pcintPort, const uint8_t pcin
#error MCU has no such a register
#endif
}
#endif
}

/*
Expand Down
96 changes: 96 additions & 0 deletions src/PinChangeInterrupt0.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,17 @@ THE SOFTWARE.
void attachPinChangeInterrupt0(void) {
// fake function to make the IDE link this file
}
#ifdef PCINT_COMBINE_PORT0_PORT1
void attachPinChangeInterrupt1(void) {
// fake function to make the IDE link this file
}
#endif

#ifdef PCINT_COMBINE_PORT0_PORT1
ISR(PCINT_vect) {
#else
ISR(PCINT0_vect) {
#endif
// get the new and old pin states for port
uint8_t newPort = PCINT_INPUT_PORT0;

Expand Down Expand Up @@ -71,6 +80,42 @@ ISR(PCINT0_vect) {
#else
PCINT_CALLBACK_PORT0
#endif

#if defined(PCINT_COMBINE_PORT0_PORT1) && (PCINT_USE_PORT1 == true)
// get the new and old pin states for port
newPort = PCINT_INPUT_PORT1;
NicoHood marked this conversation as resolved.
Show resolved Hide resolved

// compare with the old value to detect a rising or falling
arrayPos = getArrayPosPCINT(1);
change = newPort ^ oldPorts[arrayPos];
rising = change & newPort;
falling = change & oldPorts[arrayPos];

// check which pins are triggered, compared with the settings
risingTrigger = rising & risingPorts[arrayPos];
fallingTrigger = falling & fallingPorts[arrayPos];
trigger = risingTrigger | fallingTrigger;

// save the new state for next comparison
oldPorts[arrayPos] = newPort;

// Execute all functions that should be triggered
// This way we can exclude a single function
// and the calling is also much faster
// We may also reorder the pins for different priority
#if !defined(PCINT_CALLBACK_PORT1)
PCINT_CALLBACK(0, 8);
PCINT_CALLBACK(1, 9);
PCINT_CALLBACK(2, 10);
PCINT_CALLBACK(3, 11);
PCINT_CALLBACK(4, 12);
PCINT_CALLBACK(5, 13);
PCINT_CALLBACK(6, 14);
PCINT_CALLBACK(7, 15);
#else
PCINT_CALLBACK_PORT1
#endif
#endif // defined(PCINT_COMBINE_PORT0_PORT1) && (PCINT_USE_PORT1 == true)
}

#if defined(PCINT_API)
Expand Down Expand Up @@ -142,6 +187,57 @@ void PinChangeInterruptEventPCINT7(void) {
}
#endif

#ifdef PCINT_COMBINE_PORT0_PORT1
#if (PCINT_USE_PCINT8 == true)
volatile callback callbackPCINT8 = pcint_null_callback;
void PinChangeInterruptEventPCINT8(void) {
callbackPCINT8();
}
#endif
#if (PCINT_USE_PCINT9 == true)
volatile callback callbackPCINT9 = pcint_null_callback;
void PinChangeInterruptEventPCINT9(void) {
callbackPCINT9();
}
#endif
#if (PCINT_USE_PCINT10 == true)
volatile callback callbackPCINT10 = pcint_null_callback;
void PinChangeInterruptEventPCINT10(void) {
callbackPCINT10();
}
#endif
#if (PCINT_USE_PCINT11 == true)
volatile callback callbackPCINT11 = pcint_null_callback;
void PinChangeInterruptEventPCINT11(void) {
callbackPCINT11();
}
#endif
#if (PCINT_USE_PCINT12 == true)
volatile callback callbackPCINT12 = pcint_null_callback;
void PinChangeInterruptEventPCINT12(void) {
callbackPCINT12();
}
#endif
#if (PCINT_USE_PCINT13 == true)
volatile callback callbackPCINT13 = pcint_null_callback;
void PinChangeInterruptEventPCINT13(void) {
callbackPCINT13();
}
#endif
#if (PCINT_USE_PCINT14 == true)
volatile callback callbackPCINT14 = pcint_null_callback;
void PinChangeInterruptEventPCINT14(void) {
callbackPCINT14();
}
#endif
#if (PCINT_USE_PCINT15 == true)
volatile callback callbackPCINT15 = pcint_null_callback;
void PinChangeInterruptEventPCINT15(void) {
callbackPCINT15();
}
#endif
#endif // PCINT_COMBINE_PORT0_PORT1

#endif // PCINT_API

#endif // PCINT_USE_PORT0
Expand Down
2 changes: 1 addition & 1 deletion src/PinChangeInterrupt1.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ THE SOFTWARE.
#if defined(PCINT_ALINKAGE) && defined(PCINT_COMPILE_ENABLED_ISR) && defined(PCINT_INCLUDE_FROM_CPP) \
|| !defined(PCINT_ALINKAGE) || !defined(PCINT_COMPILE_ENABLED_ISR)

#if (PCINT_USE_PORT1 == true)
#if (PCINT_USE_PORT1 == true) && !defined(PCINT_COMBINE_PORT0_PORT1)

void attachPinChangeInterrupt1(void) {
// fake function to make the IDE link this file
Expand Down
4 changes: 4 additions & 0 deletions src/PinChangeInterruptBoards.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ THE SOFTWARE.
// PORTB has Reset, clock and SPI while PORTA has I2C and Analog Pins. We just enable all pins.
#define PCINT_INPUT_PORT0 PINA
#define PCINT_INPUT_PORT1 PINB
// Attiny x61 Series has a very special interrupt vector and port mapping.
// It combines PCINT0_vect and PCINT1_vect into PCINT_vect
// See datasheet section 9.1: http://ww1.microchip.com/downloads/en/devicedoc/atmel-2588-8-bit-avr-microcontrollers-tinyavr-attiny261-attiny461-attiny861_datasheet.pdf
#define PCINT_COMBINE_PORT0_PORT1 true

#else // Microcontroller not supported
#error PinChangeInterrupt library does not support this MCU.
Expand Down
6 changes: 4 additions & 2 deletions src/PinChangeInterruptPins.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,14 @@ Serial.println();
// Hardware Definitions
//================================================================================

#if defined(PCINT0_vect)
//special case for the x61 where a single interrupt vector is used for multiple ports
#if defined(PCINT0_vect) || (defined(PCINT_vect) && (defined(__AVR_ATtiny261__) || defined(__AVR_ATtiny461__) || defined(__AVR_ATtiny861__)))
Copy link
Owner

Choose a reason for hiding this comment

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

What about placing this last?

// Special check for combined ports, such as on Attiny x61
#if defined(PCINT_vect)
#ifdef PCINT_COMBINE_PORT0_PORT1
#define PCINT_HAS_PORT0 true
#define PCINT_HAS_PORT1 true
#else
#error Unknown Combined Port Configuration
#endif
#endif

This way it is more flexible.

I am also wondering if we can automate combined port detection by using #if defined(PCINT_vect) maybe? But maybe we should not overcomplicate things.

Copy link
Author

Choose a reason for hiding this comment

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

I wasn't sure whether PCINT_vect comes up anywhere else currently or might in the future that would make this check incorrect. The only reference I can find to it on other chips is for the ATtiny2313 but I'm not sure that reference is correct.

Copy link
Owner

Choose a reason for hiding this comment

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

Well if it comes up, the code compilation will fail. That is good, so I can fix the issue later on. I also think it would be a good idea to fail like this, as this gives us better detection of new, unsupported MCUs. Meaning if it uses such weird port combination we will be notified faster with this error message.

Speaking of ATtiny2313, I remember that I had lots of problems with this MCU. I might need to revisit that. But that for another PR.

Copy link
Owner

Choose a reason for hiding this comment

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

I just checked the Attiny 2313, and I am wondering why it works with ISR(PCINT0_vect)? It has no such interrupt vector (following the datasheet).

#define PCINT_HAS_PORT0 true
#else
#define PCINT_HAS_PORT0 false
#endif
#if defined(PCINT1_vect)
//special case for the x61 where a single interrupt vector is used for multiple ports
#if defined(PCINT1_vect) || (defined(PCINT_vect) && (defined(__AVR_ATtiny261__) || defined(__AVR_ATtiny461__) || defined(__AVR_ATtiny861__)))
#define PCINT_HAS_PORT1 true
#else
#define PCINT_HAS_PORT1 false
Expand Down