OnChangePin4to7 bug

[kovpeti]

Hello,

Probably i have found a bug in interrup.c in p8. OnChangePin4to7 function will be updated INTCON2bits.INTEDG2 at line 1684. This can cause improper behavior of OnChangePin2. Also the "config" parameter in definition of OnChangePin4to7 is irrelevant.

Setting the TRISB is the another problem. Just one pin will be configured as input but the name suggests all 4 will be.

If I want to use all 4 pins, i have to do something like this: pinMode(PIN1, INPUT); pinMode(PIN2, INPUT); pinMode(PIN3, INPUT); OnChangePin4to7(onPIN,PIN4,0); insted of this: OnChangePin4to7(onPIN); Of course in this case if I want to use ex: PIN2 as output, need to configure the pin as output: pinMode(PIN2, OUTPUT);

Cnsider this modified function:

//void OnChangePin4to7(callback func, u8 pin, u8 config) void OnChangePin4to7(callback func) { if (intUsed[INT_RB] == INT_NOT_USED) { intUsed[INT_RB] = INT_USED; intFunction[INT_RB] = func; INTCON2bits.INTEDG2 = config; // TRISB |= (1 << pin); // pin as an INPUT TRISB |= 0xF0; // RB7:RB4 (pin7:pin4) are INPUT INTCON2bits.RBIP = INT_LOW_PRIORITY; INTCONbits.RBIE = INT_ENABLE; INTCONbits.RBIF = 0; }

ifdef DEBUG

else
{
    debug("Error : interrupt RB is already used !");
}
#endif

}

Thanks. Peter

[rblanchot]

Hi, Thank you for this report. You're right this is not correct. Did you fix it ? Otherwise, I propose this

/ PORTB<7:4> Interrupt-on-Change Only pins configured as inputs can cause this interrupt to occur. Any RB7:RB4 pin configured as an output is excluded from the interrupt-on-change comparison. The pins are compared with the old value latched on the last read of PORTB. * pin : 4, 5, 6 or 7 * config : INT_RISING_EDGE or INT_FALLING_EDGE * NB INTCON2 : pin 0 = bit 6, pin 1 = bit 5, ... /

void OnChangePin4to7(callback func, u8 pin, u8 config) { if (intUsed[INT_RB] == INT_NOT_USED) { intUsed[INT_RB] = INT_USED; intFunction[INT_RB] = func; if (config) BitSet(INTCON2, (6-pin)); else BitClear(INTCON2, (6-pin)); TRISB |= (1 << pin); // pin as an INPUT INTCON2bits.RBIP = INT_LOW_PRIORITY; INTCONbits.RBIE = INT_ENABLE; INTCONbits.RBIF = 0; }

ifdef DEBUG

else
{
    debug("Error : interrupt RB is already used !");
}
#endif

}

[kovpeti]

Hello,

Does not need to do anything with the INTCON2 except enable RBIP bit. Actually I do not understand this: if (config) BitSet(INTCON2, (6-pin)); else BitClear(INTCON2, (6-pin)); The INTEDG bits concern to INT interrupts only (PORTB2:PORTB0).

And in your solution the TRIS setting is still wrong. I can call this function just one time, the second call will do nothing, the first evaluation will be false. This means, this example is not working:

pinMode(PIN5, OUTPUT); //PIN5 set as OUTPUT OnChangePin4to7(onPIN4,PIN4,0); //PIN4 set as INPUT OnChangePin4to7(onPIN5,PIN5,0); //expected result PIN5 is INPUT but still OUTPUT OnChangePin4to7(onPIN6,PIN6,0); //PIN6 direction is UNKNOWN

The attached interrupt routin will be onPIN4. onPIN5 and onPIN6 will never executed.

[rblanchot]

Ok for INTEDG, I mixed up INTx and RB interrupts. About TRIS : Microchip pins RB4 to RB7 are pins 4 to 7 on Pinguino and bit 4 to 7 in TRISB register. So TRISB |= (1 << pin) set pin as an INPUT without changing the other bits. I don't see any error here. For the rest you can't call different interrupt routines from OnChangePin4to7() but only one as a change on RB4 to RB7 will always trigger the same RB interrupt. The only way to know on which pin the change occurred is to read the PORTB register in your onPIN routine. Reading PORTB is also the only way to end the mismatch condition and allow flag bit, RBIF, to be cleared. So here are my changes :

lines 1702 ... void OnChangePin4to7(callback func, u8 pin) { if (intUsed[INT_RB] == INT_NOT_USED) { intUsed[INT_RB] = INT_USED; // NB : RBIF will be cleared only if PORTB has been read before // in intFunction[INT_RB]() intFunction[INT_RB] = func; TRISB |= (1 << pin); // pin as an INPUT INTCON2bits.RBIP = INT_LOW_PRIORITY; INTCONbits.RBIE = INT_ENABLE; INTCONbits.RBIF = 0; }

ifdef DEBUG

else
{
    debug("Error : interrupt RB is already used !");
}
#endif

}

lines 2003 ...

ifdef RBINT

 // NB : RBIF will be cleared only if PORTB has been read before in intFunction[INT_RB]()
if (INTCONbits.RBIE && INTCONbits.RBIF)
{
    intFunction[INT_RB]();
    INTCONbits.RBIF = 0;
}
#endif

[rblanchot]

Take care double paranthesis in #ifdef RBINT ... intFunctionINT_RB( ); .... #endif has been removed from my text by the Github Text Editor.

[kovpeti]

:) much better. I am really sorry but I still do not agree with You about TRISB. Let me explain: User wants to use RB7 and RB5 as interrupts and RB6 and RB4 as output. a;) absolute beginner user, never heard about datasheet:
OnChangePin4to7(myInterrupt,7+5); In this case the pin=12=0b00001100 -> incorrect parameter b;) intermediate user OnChangePin4to7(myInterrupt,7|5); In this case the pin=7=0b00001110 -> incorrect parameter c;) intermediate user 2 OnChangePin4to7(myInterrupt,(1<<7)|(1<<5)); (supposed bit places because of interrupt name) In this case the pin=11=0b10100000 -> incorrect parameter The working way: OnChangePin4to7(myInterrupt,(1<<3)|(1<<1)); In this case the pin=10=0b00001010 -> correct parameter Even if get a correct result this way is not clear enaught for most of user. Or: OnChangePin4to7(myInterrupt,1); pinMode(PIN5,INPUT);

Im my opinion every function has to set up every corresponding bit direction. In this function these pins are PORTB7:PORTB4. If user wants to exlude any pin, need to set as output. And this is the correct set up sequence for any project : initialise hardware modules->initialize interrupts->initialize pins->initialize variables->enable interrupts If user fallows this sequence the result will be correct.

Anyway, the origininal issue has been solved, thank You. You can close this thread. Consider my opinion as a different view point.

[rblanchot]

I think it is not a question of point of view but a question of improving/fixing the OnChangePin4to7 function and your feedback helps me a lot as I don't have enough time to check everything.

According to your example (trying to activate interrupt on change on pin 7 and 5), I thought the user could use the function like this (most of the time they will use only one pin) : OnChangePin4to7(myInterrupt, 5); OnChangePin4to7(myInterrupt, 7);

If you think an ORed version is more straight to the beginners then I could do this 1/ special pin definitions (we can't use RB4 to RB7):

define INTRB4 (1<<4)

define INTRB5 (1<<5)

define INTRB6 (1<<6)

define INTRB7 (1<<7)

2/ and clear bits 7 to 4 in OnChangePin4to7 with : TRISB &= 0x0F; 3/ and finally replace TRISB |= (1 << pin); with TRISB |= pin; Then OnChangePin4to7(myInterrupt, INTRB5|INTRB7); should work.

What do you think ?

In any case it's still up to the user to manage the pin detection in myInterrupt ... not that easy. So still something to do.

[rblanchot]

OnChangePin4to7

define INTRB4 (1<<4) // 16

define INTRB5 (1<<5) // 32

define INTRB6 (1<<6) // 64

define INTRB7 (1<<7) // 128

volatile u8 INTRB;

void OnChangePin4to7(callback func, u8 pin) { if (intUsed[INT_RB] == INT_NOT_USED) { intUsed[INT_RB] = INT_USED; intFunction[INT_RB] = func; TRISB &= 0x0F; // clears bit 7 to 4 TRISB |= pin; // pin as an INPUT //TRISB |= (1 << pin); // pin as an INPUT INTCON2bits.RBIP = INT_LOW_PRIORITY; INTCONbits.RBIE = INT_ENABLE; INTCONbits.RBIF = 0; }

ifdef DEBUG

else
{
    debug("Error : interrupt RB is already used !");
}
#endif

}

Interrupt :

#ifdef RBINT
u8 LastPortB;

if (INTCONbits.RBIE && INTCONbits.RBIF)
{
    INTRB = (PORTB & 0xF0) ^ LastPortB;
    LastPortB = (PORTB & 0xF0);
    intFunction[INT_RB]();
    INTCONbits.RBIF = 0;
}
#endif

Example :

void blink() { if (INTRB & INTRB4) toggle(0); if (INTRB & INTRB7) toggle(USERLED);
}

void setup() { OnChangePin4to7(blink, INTRB4|INTRB7); }

void loop() { }

[kovpeti]

Great! This is the perfect solution :-) maybe two additional thing:

define INTRBALL 0xF0

and: pin &= 0xF0; //just in case lower pins will not changed

I am working on a dual PID controlled DC servo (2 motors, 2 Quadrature encoders) so I will test. Thanks again!

[rblanchot]

OK done. All changes will be in the next release (Pinguino IDE v12).