Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[epsonprojector] Port of the OH 1.x epsonprojector binding **OH3 version** #7370

Closed
wants to merge 8 commits into from

Conversation

ghys
Copy link
Member

@ghys ghys commented Apr 14, 2020

This is the same OH1 binding port as #6922, but with the master branch as base.

(Full story: I'm in the process of switching my production to openHAB 3 so I needed to make this binding OH3-compatible.)

@kaikreuzer @wborn @openhab/add-ons-maintainers I don't know if it is a good time to add new bindings to the master branch yet... anyways here it is just in case, and I'm keeping #6922 open for you to decide, but I'd prefer to work on this one now since it's more difficult for me to develop & test the OH2.5 version.

This also includes the latest review comments by @cpmeister.

Signed-off-by: Yannick Schaus github@schaus.net

This is a straightforward port of the epsonprojector binding
from OH 1.x by Pauli Antilla.

Next to no changes were made to the connector code so the
binding should have the same features as the original.
Already supported Epson command types are mapped as channels.

The only missing feature is that it is not possible anymore
to configure individual polling intervals for each command
type/channel; there's a global polling interval only.

Signed-off-by: Yannick Schaus <github@schaus.net>

# Conflicts:
#	bom/openhab-addons/pom.xml
#	bundles/pom.xml
Signed-off-by: Yannick Schaus <github@schaus.net>
Signed-off-by: Yannick Schaus <github@schaus.net>
Signed-off-by: Yannick Schaus <github@schaus.net>
Signed-off-by: Yannick Schaus <github@schaus.net>
Signed-off-by: Yannick Schaus <github@schaus.net>
openhab#6922 (review)

Signed-off-by: Yannick Schaus <github@schaus.net>
@TravisBuddy
Copy link

Travis tests were successful

Hey @ghys,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@kaikreuzer
Copy link
Member

If you are also finishing #6922 and we soon have it in the 2.5.x branch, it will be automatically ported to 3.0.
If you do not plan to do any (bigger) further evolution of #6922 after it is merged, I'd suggest to only keep 2.5.x and close this PR here.
If you want to continue working on that binding and you want to do so on 3.0, then better close #6922 and have the binding only available for 3.0.

@ghys
Copy link
Member Author

ghys commented Apr 14, 2020

I was switching the serial transport from gnu.io to org.openhab.core.io.transport.serial (which is org.eclipse.smarthome.io.transport.serial in OH2), as requested, but was doing it "in the blind" since my development environment is mostly OH3 and my instance too, so that led me deciding to do the other package renames and have a proper OH3 version that I could test in production - so I'm not frankly interested in getting it working on OH2 anymore ;).

Also figured maybe you'd want this to be a test of having to deal with both OH2 bindings to port and OH3-exclusive ones at the same time, since there could be more of that in the future?

@wborn
Copy link
Member

wborn commented Apr 14, 2020

I'm fine with this being a OH3 only add-on for now. It's nice to see the serial transport being used as well. 👍 The downsides of this being an OH3 only add-on are of course that there will be less users who can easily test it. Also, if they can already migrate to it while being on OH2, they have one less manual migration to do whenever they upgrade to OH3. 😉

@kaikreuzer
Copy link
Member

I'd say the decision is absolutely up to @ghys - both options are imho fine for us.

@ghys
Copy link
Member Author

ghys commented Apr 14, 2020

Perfect, then I'm fine with it being the first OH3-only binding, if everything's fine wrt the infrastructure :) Don't worry about the testing, it's not the most used binding I suppose, so I can take care of the testing myself since I happen have plenty of opportunities to do so these days, for some reason... No rush in merging it, I can use it for a few days/weeks to confirm it's stable first.

Signed-off-by: Yannick Schaus <github@schaus.net>
@TravisBuddy
Copy link

Travis tests were successful

Hey @ghys,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@J-N-K
Copy link
Member

J-N-K commented Apr 15, 2020

@ghys I don‘t want to disappoint you, but #6678 is another OH3-only port of an OH1 binding 😀

@J-N-K J-N-K added the oh3 label Apr 15, 2020
@ghys
Copy link
Member Author

ghys commented Apr 15, 2020

I stand corrected then 😄

@billfor
Copy link
Contributor

billfor commented Apr 18, 2020

If you are also finishing #6922 and we soon have it in the 2.5.x branch, it will be automatically ported to 3.0.

Will the automatic conversion be available as a script for bindings that are not in 2.5.x? I have my own binding that I did not submit but it works with 2.5, and I might like to use it in 3.0 when it comes out.

Thanks.

@kaikreuzer
Copy link
Member

Could you please re-create this PR against the main branch?
See point (4) in #8512. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants