cyborg5 / IRLib

An Arduino library for encoding and decoding infrared remote signals
Other
213 stars 76 forks source link

Merging efforts between IRLib and Arduino-IRremote #13

Closed z3t0 closed 6 years ago

z3t0 commented 9 years ago

@cyborg5

Hi,

I am the current maintainer of Arduino-IRremote and would be interested in merging efforts between the two repositories. Please check all the links below as well.

Links:

https://github.com/z3t0/Arduino-IRremote/issues/199 https://github.com/z3t0/Arduino-IRremote/issues/200 https://github.com/z3t0/OpenIR https://github.com/z3t0/SmartIRLib.git

bengtmartensson commented 9 years ago

I was using IRLib in AGirs, but found that it did not satisfy my requirements. So I made my own version of some of the classes therein, now contained in the AGirs project. What I did not like with IRLib:

Although packaged as classes, it is not really an OO design. The reader is advised that he is not "supposed" to instantiate more than one instance of a receiver and one instance of a sender at a time, but that is both overkill, and a good OO design should enforce it. Also, in the classes, (almost?) everything was public... No support for runtime changeable timeouts (I wrote a pull request for this). No support for non-modulated sending. And I did not like putting all the classes in one single file (yes, I have a Java background :-)). So I wrote my own classes, partially recycling the classes of IrLib, now found in AGirs/src/GirsLib, addressing these shortcomings. For example, classes that are only to be instantiated once are implemented as singleton classes, with no public constructor, but instead a static factory method that inforces the Highlander principle ("there can only be one"). Please consider these as candidates in the merging. (AGirs/src/GirsLib contains more than that, but that is not relevant for this discussion.)

cyborg5 commented 9 years ago

I appreciate the feedback. I will take a look at the runtime changeable timeouts and will probably make that an option. Among the major changes for version 2.0 will be splitting out each protocol into its own separate sets of files and then having a master file that will merge together only the protocols you wish to use. I think I have a pretty clever way of doing that worked out. I'm also interested in your methods for focusing a single instance of a class. When I get time I will check that out.

From: Bengt Martensson [mailto:notifications@github.com] Sent: Monday, August 17, 2015 4:42 AM To: cyborg5/IRLib IRLib@noreply.github.com Cc: Chris Young cy_borg5@cyborg5.com Subject: Re: [IRLib] Merging efforts between IRLib and Arduino-IRremote (#13)

I was using IRLib in AGirs https://github.com/bengtmartensson/AGirs , but found that it did not satisfy my requirements. So I made my own version of some of the classes therein, now contained in the AGirs project. What I did not like with IRLib:

Although packaged as classes, it is not really an OO design. The reader is advised that he is not "supposed" to instantiate more than one instance of a receiver and one instance of a sender at a time, but that is both overkill, and a good OO design should enforce it. Also, in the classes, (almost?) everything was public... No support for runtime changeable timeouts (I wrote a pull request for this). No support for non-modulated sending. And I did not like putting all the classes in one single file (yes, I have a Java background :-)). So I wrote my own classes, partially recycling the classes of IrLib, now found in AGirs/src/GirsLib, addressing these shortcomings. For example, classes that are only to be instantiated once are implemented as singleton classes, with no public constructor, but instead a static factory method that inforces the Highlander principle ("there can only be one"). Please consider these as candidates in the merging. (AGirs/src/GirsLib contains more th an that, but that is not relevant for this discussion.)

— Reply to this email directly or view it on GitHub https://github.com/cyborg5/IRLib/issues/13#issuecomment-131732903 . https://github.com/notifications/beacon/ADcgulZOENSSqcV4TdlJFbvloUDP62L7ks5ooZXtgaJpZM4FsSYM.gif

q2dg commented 9 years ago

Well...what about original proposition? It would be really great if merging is finally done! One library to rule them all!!

z3t0 commented 9 years ago

@q2dg I agree, however as it stands I do not have a lot of time on my hands... so If anyone would be willing to take the initiative?

bengtmartensson commented 9 years ago

What do you mean by "merge"?

Chris' library can be considered as a branch of IRremote, so it is natural to expect it to be "merged in". But since it reorganizes things, packs things in classes etc., that would mean that the API would change completely, and everything that used the old IRremote would have to be rewritten for the post-merged version. (Then Chris is encouraged to cease developing his version, instead branching/forking IRremote.) Is that what is desired? Then there is of course my changes, described above...

Or is it desired to produce the non-plus-ultra library, borrowing from IRremote and IRLib, surpassing all others in functionality, thus replacing them?

Any way, the problem with those users who was told by their roommate's sister-in-law's nephew's girlfriend to use Shirriff 2009 library remains...

cyborg5 commented 9 years ago

I have always considered IRLib to be its own entity apart from IRremote while acknowledging that it was based upon IRremote. It wasn't just a patch or edit. It was a major rewrite with a much different design philosophy. I will leave it up to the end users to decide whether or not my redesign has merit. I suppose people will use whichever one they like best. If there are any protocols in IRremote that I don't support I would welcome someone submitting them in a form compatible with the rest of my work. And if those who work on IRremote wish to create protocols for that library based upon my work I have no problem with that either.

The only areas that really would lend themselves to some sort of merge would be the hardware specific defines. Initially my hardware specific code was lifted directly from a branch of IRremote except that I moved some out around if two different files but the code itself was pretty much identical. Then when I tried to implement bit-bang PWM and other attempts to make a less hardware dependent option, it made sense to be to separate the hardware defines for receiving from the hardware defines for sending. In the process of doing that rewrite I broke some of the code especially for the early teensy boards.

What I would welcome is having IRremote and IRLib have identical hardware specific portions again. But I really need the sending and receiving portions independent of one another. I have access to Arduino Uno, Leonardo, Mega, Micro and the now defunct Pinocchio platform. Anything other than that I cannot test myself. So I would welcome someone who has access to a broader range of hardware to test and patch my IRLibTimer.h while maintaining its basic new structure. If IRremote would then adopt that file identical to mine that would be useful.

I know it sounds a bit egotistical of me to say in a way "do it my way or else". However open source doesn't necessarily mean willingness to give up control of one's own code for one's own purposes. I did the design the way I felt was right for my purposes and I believe that others agree.

I'm currently working on IRLib 2.0 which includes another pretty big restructuring. Basically each protocol is going to have its own whatever.cpp and whatever.h files. You can then add or remove protocols simply by only including the whatever.h files for the protocol you want to use. This particular rewrite will not include any significant changes to IRLibTimer.h so any work that is done updating that file won't hurt anything I'm doing in their future.