sui77 / rc-switch

Arduino lib to operate 433/315Mhz devices like power outlet sockets.
1.92k stars 663 forks source link

Multiple receivers #71

Open Ernest314 opened 8 years ago

Ernest314 commented 8 years ago

I'm currently working on a project that requires multiple receivers (at 433MHz and 315MHz). I've read through past issues regarding this (#25 and #63), and I have a proposal for (sort of) resolving this issue.

What I'm planning on doing is to add the functionality to query which pin triggered the interrupt, which would at least allow two receivers to be used. This would also probably require changing variables such as nReceiverInterrupt to dynamically-allocated arrays, or better yet std::vector<int>s. (I'm working on a Particle Photon, so I can use STL libraries.) Would this be a satisfactory way to address this issue? I am currently working on a version that does use <vector>, but I assume with some effort that can be ported to malloc and such.

Please tell me if I've overlooked anything (or I'll find out in due time, I suppose).


Edit: The naive implementation would make the assumption that two pins would never be receiving simultaneously.


Edit 2: After working on this some more, it appears that it could substantially increase the size of the library (e.g. ideally there would be an enable_receive(std::vector<int>) etc.). Is this a problem? I suppose something could be done with #ifdefs and such.


Edit 3: I suppose multiple receivers operating at once might be possible if all of the static variables were turning into vectors/arrays? This depends on how the receiving protocol works, which I haven't figured out yet :P

Ernest314 commented 8 years ago

After considering how to port my changes, perhaps it would be better to have a macro #define for the max number of receiver pins a user might require? This would simplify things a lot, and appears to be an appropriate use for a #define.

fingolfin commented 8 years ago

In principle, I think it would be nice to support more than one receiver, if this does not impose a penalty for people using a single receiver or no receiver at all, and if it is done well :-).

Using std::vector (or any other kind of dynamically resized arrayed) sounds like both unnecessary, and also bad from a technical view point.

It is unnecessary, as the number of supported receivers would likely need to be hardcoded at compile time anyway (to facilitate the interrupt handlers, one per input source). Moreover, the developer using rc-switch will know how many receivers there are going to be anyway.

It has technical drawbacks, because any form of dynamic memory management incurs a severe resource overhead (severe at least for restricted platforms like Arduino, ATTiny etc): it uses up more RAM increases the binary size (i.e. needs more flash), increasse CPU requirements, increases the overall complexity (and thus likelihood for bugs -- e.g. if an interrupt handlers accesses a std::vector, it must not be resized during that time -- so locking issues may need to be considered).

And even if somebody is willing to make all these tradeoffs for their particular use case, it seems like a bad idea to burden existing users with all that overhead when they don't need it.

Luckily, all of that is not necessary. Using #define macros and some clever coding, one can allow support for multiple receivers with little work, little overhead, and no impact at all on people who only need a single receiver. I very roughly sketched a plan for that on issue #63.

Ernest314 commented 8 years ago

Fortunately, it turns out that's what I eventually ended up doing :)

Unfortunately, I was modifying Particle's version of the library (found here), which is horrifically outdated (and unformatted). I do have a working implementation right now that I can send to you, but I will need a bit to update it to work with the current version of the library.

fingolfin commented 8 years ago

Well, I don't really have a need for it myself, so even if you sent me something, I wouldn't do anything with it. But feel free to push whatever you have already to a github repo, and post a link to it here, then I can perhaps give some feedback already. Otherwise, if you at some point in the future manage to update it to the latest rc-switch, we'd definitely welcome a pull request for this feature :-).

Ernest314 commented 8 years ago

I pasted a quick version of what I have so far here, complete with debugging statements and commented out code. :P I'm currently working on cleaning it up and testing it, after which I will work on updating it to what the library currently uses... Then I'll be able to submit a proper pull request.

Or maybe I should just fork the current repo and port over the changes. I'm not sure which is less work.