Make it more platform independent

[lucadentella]

Hi

I was trying to use this library with esp32 and it can't compile due to the dependency with InterruptHandler library which requires avr.h (same problem in #3 ).

If I understood correctly, the library is needed to have the possibility to call class members as ISR... I was able to modify the library (see here my fork) to remove the dependency and implementing a different approach (explained here).

Before asking for a PR, @Nikkilae I'd like to know your opinion about this...

Thanks!

[lucadentella]

Hi @Nikkilae

did you have time to review my code? Are you ok if I create pull request or do you prefer to keep you code as is? Thanks

[Nikkilae]

Hi,

Sorry, I read your message earlier but admittedly totally forgot about it.

Your solution will work if you only use one PPMReader at a time - always the one that was most recently constructed. Otherwise it is not a robust solution because effectively you can't use multiple PPMReader instances at the same time, which defeats the purpose of making this thing object oriented. That's why I would not merge the pull request if you made it with that solution.

I think that you have the right idea though, and I believe that there is a similar relatively simple solution that would work while retaining the object orientedness of the library: Have a global ISR for each pin and have a global data structure that maps a pin to the PPMReader that is listening to it. In PPMReader's constructor update the map so that the pin number of the reader points to that reader. Each of the pin specific ISR's should look at the map and call the corresponding PPMReader's handleInterrupt(). This solution assumes that for one pin there can be only one PPMReader but I think that's a reasonable restriction.

This shouldn't be a very complicated implementation but I'm not sure when I would have a chance to implement this because I don't have the hardware to be able to test it right now. When I wrote this library initially four years ago I didn't really expect to revisit it :) If you or someone else find that my proposal would work, feel free to implement it. Or if you figure out some other solution, of course. If you're also able to test that the solution works with multiple readers on different pins, make a pull request and I'll take a look.

[lucadentella]

Hi,

thanks for your reply, you're absolutely right about my solution, which leads to singleton classes.

Are you thinking about something similar to what Paul implemented here? https://github.com/PaulStoffregen/Encoder

I must admit I'm not a big fan of all those "ifdef" to create the correct number of static ISR depending on the specific board/chip. I'm wondering if there's a more "elegant" way to do it... maybe using class templates?

[Nikkilae]

I agree that the approach of board specific preprocessor directives is not appealing. A quick glance at the arduino reference reveals no way of determining the number of pins at compile time either, but if there is one that I missed, it should be used if my earlier idea were to be implemented.

I'd imagine that a relatively clean solution is possible where the pin number is given as a template argument and a static ISR is generated using that. This would of course be a breaking change to the library which I'm not so keen on making. I do think that it's a worthwhile endeavor, but at that point there would not be much left of this library as it is in its current state so I'd rather make it a completely new library.

[lucadentella]

Thanks for your response! I totally agree: at this point a new library is preferable. This said, I'm closing the issue for now.

[i998]

Hi All, I made a version of the PPMReader for STM32 for my FlyByWire project - you can see it here https://github.com/i998/FlyByWire/tree/master/src

Thanks to Nikkilae for the original code!