SpenceKonde / ATTinyCore

Arduino core for ATtiny 1634, 828, x313, x4, x41, x5, x61, x7 and x8
Other
1.53k stars 301 forks source link

Is there an equivalent of TinyPinChange (that exists on the Digispark Pro repo) for the Standalone Attiny167? #792

Closed Rimbaldo closed 9 months ago

Rimbaldo commented 11 months ago

Hi!

I need a rising PinChange interrupt on a not external interrupt pin on my barebones Attiny167. I need it on PCINT2

There's a Library for the Digispark PRO (from the Digistump repo) that handles any pinchange interrupt, not only on change, but also Rising, Falling edges on specific pins, not all pins of the same port. Extract from the TinyPinChange.h below:

* a library for Pin Change Interrupt by RC Navy (2012)
* Supported devices: ATmega238P (UNO), ATmega2560 (MEGA), ATtiny84, ATtiny85, ATtiny167
* 
* http://p.loussouarn.free.fr
* 20/04/2014: Support for MEGA added
* 22/12/2014: Support for ATtiny167 added
*  Methods TinyPinChange_Edge(), TinyPinChange_RisingEdge(), TinyPinChange_FallingEdge() added
*  TinyPinChange_GetPinEvent() replaced with TinyPinChange_GetPortEvent()
*  TinyPinChange_GetPinCurSt() replaced with TinyPinChange_GetCurPortSt()

This TinyPinChange library (and it's examples - this one above is from the DigisparkRisingEdge Example) is available only for the Digispark Pro board from the Digistump board manager, on the Arduino IDE.

Does Attiny core supports it? If not, can the code from Digispark Pro be adapted for the Attinycore? Or is there a way I can use this library with the Attinycore, in my barebones attiny167?

Thanks, Rodrigo

SpenceKonde commented 11 months ago

Since people often find long posts of mine too timconsuming to read (hmmph, try writing then!) I've bolded the most important things to read.

Beats me, please link to the exact library so I can tell you (my policy is that I do not investigate any questions about any library without the URL to the library the user is working with, because any library published today has been forked multiple times and exists in several versions in various places online (sometimes hundreds of versions of libraries with an identical name can be found) 90% of which have not described the changes or even that anything has been changed. The name of a library is insufficient to identify the library.

HELL! EVEN I MAINTAIN TWO LIBRARIES OF THE SAME NAME BUT DIFFERENT API - Flash.h on mTC was written by another user, Flash.h on DxC was written by me.both let an app write to the flash. Everything else about them is completely and totally different. Dx and tiny parts are different, of course, and so some of the API differences are a reflection of that, and some of the fact that the Dx had like no bootloader space left for spm from app except by the simplest means possible. But most is just that we have very very different coding philosophies and both think that the other's way is either bad or stupid or evil. What he thinks to be wicked and hazardous, I see is beautiful and powerful, what I see as ineffective and profane, he sees as mandatory and sacred. The coding constructs he likes, I can't read (he's really good at C++) but I throw inline asm around now that I'm decent at it and I don't think he's good at reading that He is even of the heretical view that functions should be named lower_case_with_underscores(); instead of camelCaseLikeGodIntended() (crowd: gasp "Burn him!" )

What I'm saying is, A library cannot be uniquely identified by it's name and version number, it must be identified by the URL it obtained from, or by identifying it as the being in library manager and of a specific version. If I search for that, I'm going to get a bunch of results forks of it, the code will not be the same, the differences are unlikely to be described, and I won't know which one you're using. I have seen examples so extreme that the only things they had in common that were user facing were the name of the library and they part they worked with. The API was completely and totally different I got burned by that bad - lost the old library in an hdd crash and then thought I'd redownloaded it... nope. Tried half a dozen of the most popular ones, but none of them had an API that was remotely similar, and I had to reimplement my code with a new library because I couldn't find the one I'd used before which had a totally different API...

That library may or may not be compatible and I could tell quickly by looking at the code were you to link me to it.

That said, without reading the library source code, just knowing the limits of the hardware, I can tell you that the performance of that library will be markedly worse than using a manually defined INTn interrupt. Actually, i'm pretty sure that it can be implemented for a specific case using a PCINT with only a single pin within a port with an overhead of just like 5-6 clocks more than an INTn interrupt. But any library has to serve the general case. The general case, which is what the library has to implement has to include a mechanism of setting a function pointer to call and then calling it from the ISR, in addition to logic for checking for changes on all 8 pins. Not having read their code (can't do that until you link it), I'm gonna spitball... 120-160 clocks plus the execution time of the body of the ISR

This is what the INT0 and INT1 pins are for, I would advise reassessing why both of these are unavailable to you, because you would get a much more performant result with fewer compatibility issues with other libraries and code.

The authorship of libraries that use PCINTs - and really any implementation of the concept of "attaching an interrupt" - is an evil act. On the first count, because it's generally not possible within the Arduino API or a framework with similar concepts without grabbing on to EVERY PCINT vector so you can't can define one manually. (It's even worse on modern AVRs, which don't have INTn interrupts, they just give every pin's PCINT rising and falling options, so you use attachInterrupt() once and you cannot use any manually defined pin depent interrupts, even an empty one because you're just waking from sleep with it. I busted my ass trying to fix this on modern AVRs - I can now get it to only grab onto some of the port interrupts.... but I just can't seem to get it to detect which ones it needs, even when the pin arguments to attachInterrupt are constant. That took a HUGE amount of work and I was fixing bugs like like 4-6 months... and the benefits are less than I was hoping to see. I could feel the prize slip from my fingers at the end.... an incredible disappointment. It would have been transformative, too!) On the second count attached interrupts perform miserably - depending on register pressure in the function (which will adversely effect performance in the second example), the first example will be 9 to 57 clock cycles slower (ie, a well written manually defined interrupt may well finish before the prologue is even finished for the attachInterrupt defined one - that's the difference between:

ISR(some_vect) { 
*funcpointer();
}
// assuming function is just ret (empty function), this takes about 3+3+8+24+3+4+24+7+4 = 80 clocks (on modern AVRs, subtract 16 clocks - push is 1 clock instead of 2, and the two call-like operations are 1 clock faster because it can push the return address onto the stack faster)

instead of

ISR(some_vect) {
//insert body of pointed to function here
}
// assuming empty body this takes 3+3+8+7+4= 25 clocks (subtract 5 on modern AVR) ).  For the case where this literally is the ISR, ie, it's an empty ISR for waking from sleep or one that uses no working registers and performs no logical or arithmetic functions (things that would change the SREG), the ISR can be stark naked and have a runtime of only 3+3+4 =  (subtract 1 on modern AVR) 10 clocks from interrupt triggering to resumption of interrupted code... 

Note that modern AVRs do lose 1-5 clocks relative to classic AVRs like these since on the newer parts you usually need to explicitly clear the intflag

The interrupt vectors have to be static places in the program space - there is a jmp or rjmp instruction at a specific location in the flash which jumps to the interrupt handler (that has to be compile time known). The only way to associate code with an ISR at runtime is to define the ISR() is to put the logic that happens to decide which ISR to call (if applicable, which it is here) into the ISR, and have it call a user-supplied function pointer. Whenever a C function is called in an ISR and cannot be inlined (function pointer calls never can be), it gets whacked by brutal overhead in order to ensure that the ABI is maintained (because the function is allowed to change 12 additional registers (in addition to the normal 2) that must be preserved in an ISR context, we don't know how many of those whatever function we get pointed to will need, so the compiler has to save and restore all of the, and on a classic AVR that's 2 clocks each to save, 2 clocks each to restore So that's 28 at the start and 28 at the end, on top of the 8 at the start (push r0, in r0, SREG, push r0, push r1 eor r1, r1) and 7 at the end (pop r1, pop r0, out SREG, r0, pop r0) and the 3-4 clocks to enter the ISR and 4 clocks to return from it. Plus there may be logic needed to decided on which pointer to call. (in the case of your pcint library, it's triggering every time there's a change of either sort on the pin, reading the pins, then looking for pins that have changed, and whether they changed into a 1 or a 0, and whether that matches an interrupt; That's a sizable overhead within an ISR context.

So that library, whether or not it is supported, will never perform well (and in fact will always be slower versus attachInterrupt, which already sucks).

SpenceKonde commented 11 months ago

Updated with a bit more through comment

Rimbaldo commented 11 months ago

Ok. Thanks for the explanations! and I'm sorry for the long delay...

The library was not in the normal arduino "libraries" folder. It's examples only appears when the Digistump core is selected.

So I navigated to the Digistump core folder and copied the "DigisparkTinyPinChange" folder library to the arduino library normal folder.

Then It seems to work right out of the box! It compiled normally. I'll try later to see if really works with my barebone Attiny167! I'll let you know later.

If needed, I can post the whole CPP and header files here, If you think it's worth to take a look!

I'll get back later!

Regards, Rodrigo

SpenceKonde commented 10 months ago

Well seeing as I have no idea what this library is supposed to do it's hard for me to even make much of a comment, yaknow?

Rimbaldo commented 10 months ago

Hi! I'm sorry!

I gave up of trying to make it work and used the direct registers approach to call the pin change interrupts...

I managed to modify a Library, called PinChangeInterrupt Library https://github.com/NicoHood/PinChangeInterrupt, to make it work in the Attiny167 (it doesn't support the Attiny167) but in the end, the direct approach was easier, and I stopped using the library.

I must be the only person that has bought some Digisparks PROs and some bare 167 chips for my projects... I like using them, but they are not very well supported in terms of third party libraries... (except for your super core!)

SpenceKonde commented 9 months ago

The problem, and it is one that was encouraged by atmel and discouraged by Microchip as a sideffect of their development paradigms, is that classic AVRs are more heterogeneous, and are presented in their documentation as even moreso. There is neither rhyme nor reason evident in much of the classic AVR design. How did they assign those alternate functions? Did they hang up a pin diagram on a dart board and choose that way? Did they unroll the diagram on a table when the semiconductor seer entered the room, cast a handful of bones onto it, peered at them for a few moments, announce the mapping and walk off as quicky as he arrived? And then, on top of that, people like digispark build a castle out of the damp sand of arduino's already yucky API by apparently competing with eachother to see who could make the arduino pin mapping make the least sense and posess the fewest mathematical shortcuts. Unfortunately they picked the winner of that competition instead of the loser. But in any event, in the past, it was hard to write code that worked on lots of AVRs because stuff got moved around seemingly at random. How do you deal with that as a library author?!

You explicitly test for the most popular parts as say "f the rest"! And that's what most libraries do!

Modern AVRs are more homogenous. You can generalize the vast majority of code between parts. There are two sets of pinouts - the tinyAVR pinouts for n = 8, 14, 20, 24. And the non-tiny pinouts for n = 14, n = 20, n = 28, n = 32, n = 48, and n = 64, and very simple rules for how they change with MVIO. I have cores for both. Almost every library is the same between the two cores, as is the majority of the API code. and within non-tiny or tiny, as long as you use PIN_Pxn notation to reference pins as we recommend, sketches will often "just work" on a large number of parts!

At a pincount of 64, by the way, modern AVRs are at the limit of what any AVR can do gracefully. Up to 56 I/O pins and 128k of flash and AVR is well within it's zone of effectiveness. Any more flash, and it abruptly gets slower, and any additional pins are second class pins (they're out of VPORT registers at that point). So what will they do to provide the migration path from the m2560? Probably... the same thing.... they did before....? a huge number of second class pins with no bit access and a nonchalant shrug to concerns about slower calls? Gee, I hope they can manage something better than that.... but they kinda are stuck. As soon as 16 bits can't store the program counter, call and ret have to shift an extra byte on or off the stack, so they're slower. Not much slower, but slightly. Depends on how much the code calls functions internally (and that are actually implemented by the compiler with call and ret, as opposed to being inlined or something. Not nearly as bad IMO as the second class pin issue. That one is fixable but only by means so over-the-top I'll eat my hat if they do it.