Closed GoogleCodeExporter closed 9 years ago
[deleted comment]
[deleted comment]
[deleted comment]
I have a EH-TW3600 which has no ethernet-connection.
Will this binding be for ethernet or serial-connections?
Original comment by christop...@gmail.com
on 13 Jul 2013 at 7:59
Binding is for serial port communication, but code stub for ethernet-serial
port adapter is also generated but not implemented yet.
I have developed binding against EH-TW3200 which is pretty much the same
projector.
Original comment by pauli.an...@gmail.com
on 14 Jul 2013 at 7:48
That's really great!!! :)
I think I need an USB-to-serial-adapter. :)
Original comment by christop...@gmail.com
on 14 Jul 2013 at 8:09
[deleted comment]
Original comment by teichsta
on 6 Aug 2013 at 10:17
[deleted comment]
Hi Pali,
thanks for contributing this binding. Please find below my Review remarks:
# EpsonProjectorBindingProvider
* isInBinding() seems to be never called. Is this method superfluous probably?
* if isInBinding is superfluous getRefreshInterval is superfluous as well
* if isInBinding is superfluous getInBindingItemNames is superfluous as well
# EpsonProjectorCommandType
* since this enum seems to belong to the "internal" data structure it should be
moved in the .internal package
* L84 ',' is superfluous
* L122-126 please use curly brackets although not really necessary but it
supports better readability
# EpsonProjectBinding
* L114-135 please move code to a separat method disconnectAll() (name is just a
proposal)
* L297-298, L342-343 please remove commented code
* L683-688, L708,709 since EpsonProjectorTcpConnector is not implemented and
should be removed hence these parameters could be removed as well
# EpsonProjectorConnector
* this class seems to be better represented by an interface
* when changed to Interface, please remove 'public abstract' prefix
# EpsonProjectorDevice
* L361 please remove constructor for TcpConnector
* L365, L389 String comparison should be done with equals()
* L442, L489, L515, L531, L633, L759, L806, L822, L838, L858, L871, L895,
which exception could be thrown/catched here?
* L923 ErrorStrings should be externalized (see novelanheatpump binding if you
need details how to accomplish i18n)
* L990 IF board > If board
# EpsonProjectorGenericBindingProvider
* L108 remove inBinding member if isInBinding is superfluous
* L276 member seems not to be filled anywhere
* L280 remove inBinding if not necessary
# EpsonProjectSerialConnector
* L104, 107 you could better use commons.IOUtils to close handle Streams more
comfortable. It contains also methods to open and administer streams
# EpsonProjectorTcpConnector
* please remove this class since it seems to be a place holder for future (when
ever) developments
Best regards,
Thomas E.-E.
Original comment by teichsta
on 7 Aug 2013 at 10:06
Thanks for the review Thomas.
# EpsonProjectorBindingProvider
* isInBinding() seems to be never called. Is this method superfluous probably?
* if isInBinding is superfluous getRefreshInterval is superfluous as well
* if isInBinding is superfluous getInBindingItemNames is superfluous as well
Binding provide both in and out functionality, so isInBinding() is not
superfluous. Well isInBinding() method is not used, because interface provide
also getInBindingItemNames() method.
# EpsonProjectorCommandType
* since this enum seems to belong to the "internal" data structure it should be
moved in the .internal package
Done. I have copied this "idea" from other bindings, so your probaply should
clean up other bindings on the main repo as well.
* L84 ',' is superfluous
Well, it's fully supported by the Java so I don't agree that it's superfluous
:) There is good reasons why it's supported:
1) If use similar style as I use, diff only show newly added line(s).
2) Easier to add and comment out some enum values.
3) Make life easier to code generators.
* L122-126 please use curly brackets although not really necessary but it
supports better readability
Done.
# EpsonProjectBinding
* L114-135 please move code to a separat method disconnectAll() (name is just a
proposal)
Done.
* L297-298, L342-343 please remove commented code
Done.
* L683-688, L708,709 since EpsonProjectorTcpConnector is not implemented and
should be removed hence these parameters could be removed as well
Are your really sure that you want me to remove TCP connector.
It's much more easier to someone add TCP support, because all functionality is
on already on place.
My installation is currently so that I can use serial port connection to my
projector, but I'm pretty sure that most of the users needs e.g. tcp-serial
port adapter, because openhab is located to different place than projector
itself.
# EpsonProjectorConnector
* this class seems to be better represented by an interface
* when changed to Interface, please remove 'public abstract' prefix
Done.
# EpsonProjectorDevice
* L365, L389 String comparison should be done with equals()
Done. I do the same mistake every once in a while (C/C++ has been my primary).
* L442, L489, L515, L531, L633, L759, L806, L822, L838, L858, L871, L895,
which exception could be thrown/catched here?
Fixed.
* L923 ErrorStrings should be externalized (see novelanheatpump binding if you
need details how to accomplish i18n)
Done. That's really nice feature.
* L990 IF board > If board
"IF" is abbreviation which is copied directly from Epson spec. No clue what it
means.
# EpsonProjectorGenericBindingProvider
* L276 member seems not to be filled anywhere
Fixed.
# EpsonProjectSerialConnector
* L104, 107 you could better use commons.IOUtils to close handle Streams more
comfortable. It contains also methods to open and administer streams
Done.
Original comment by pauli.an...@gmail.com
on 8 Aug 2013 at 6:38
I got a USB-to-Serial-Adapter, a long cable and an epson projector.
All I need to test the binding are a compiled binary and some time.
You can help me with the first one. ;)
Is there any other documentation than the list of commands in the source?
https://code.google.com/r/paulianttila-ihc-binding/source/browse/bundles/binding
/org.openhab.binding.epsonprojector/src/main/java/org/openhab/binding/epsonproje
ctor/internal/EpsonProjectorCommandType.java
Maybe you can post some parts of your items-file.
Original comment by christop...@gmail.com
on 10 Aug 2013 at 8:26
Original comment by kai.openhab
on 11 Aug 2013 at 7:32
Preliminary Wiki page can be found here
http://code.google.com/p/openhab/wiki/EpsonProjectorBinding
Original comment by pauli.an...@gmail.com
on 11 Aug 2013 at 8:16
Attachments:
I was just a little playing with the new binding and got this:
18:24:07.343 DEBUG o.o.b.e.i.EpsonProjectorDevice[:358]- Query: 'SOURCE 20'
18:24:07.545 DEBUG o.o.b.e.i.EpsonProjectorDevice[:361]- Response: ':'
18:24:31.445 DEBUG o.o.b.e.i.EpsonProjectorDevice[:358]- Query: 'PWR?'
18:24:31.547 DEBUG o.o.b.e.i.EpsonProjectorDevice[:361]- Response: 'PWR=01'
18:24:31.547 DEBUG o.o.b.e.i.EpsonProjectorBinding[:199]- item 'epsonSource' is
about to be refreshed now
18:24:31.547 DEBUG o.o.b.e.i.EpsonProjectorDevice[:358]- Query: 'SOURCE?'
18:24:31.648 DEBUG o.o.b.e.i.EpsonProjectorDevice[:361]- Response: 'SOURCE=21'
18:24:31.649 WARN o.o.b.e.i.EpsonProjectorBinding[:372]- Couldn't execute
command 'Source',
org.openhab.binding.epsonprojector.internal.EpsonProjectorException: Can't
convert value33 to Source
18:24:31.649 ERROR o.o.b.e.i.EpsonProjectorBinding[:213]- No response received
from command 'Source'
Original comment by christop...@gmail.com
on 11 Aug 2013 at 4:26
Currently only
COMPONENT(0x14), PC_DSUB(0x20), HDMI1(0x30), HDMI2(0xA0), VIDEO(0x41),
SVIDEO(0x42) are supported on source.
You can use DirectSource (number item) and then introduce mappings on the
sitemap.
Original comment by pauli.an...@gmail.com
on 11 Aug 2013 at 5:03
Ok.
I get another error in the logs:
20:57:53.982 DEBUG o.o.b.e.i.EpsonProjectorBinding[:199]- item
'epsonHorizontalReverse' is about to be refreshed now
20:57:53.983 DEBUG o.o.b.e.i.EpsonProjectorDevice[:358]- Query: 'HREVERSE?'
20:57:54.083 DEBUG o.o.b.e.i.EpsonProjectorDevice[:361]- Response:
'HREVERSE=OFF'
20:57:54.084 WARN o.o.b.e.i.EpsonProjectorBinding[:372]- Couldn't execute
command 'HorizontalReverse',
org.openhab.binding.epsonprojector.internal.EpsonProjectorException: Illegal
response
20:57:54.084 ERROR o.o.b.e.i.EpsonProjectorBinding[:213]- No response received
from command 'HorizontalReverse'
But everything works as expected.
So I don't know, what the error is.
Switch epsonHorizontalReverse { epsonprojector="hometheater:HorizontalReverse:ON,60000" }
Switch item=epsonHorizontalReverse label="Horizontal Reverse"
Original comment by christop...@gmail.com
on 11 Aug 2013 at 7:00
I finidhed builduing a little demo.
https://code.google.com/p/openhab/wiki/EpsonProjectorBinding?ts=1376249007&updat
ed=EpsonProjectorBinding
Seems all good.
But I think the strings for the ErrMessages are missing. ;)
Original comment by christop...@gmail.com
on 11 Aug 2013 at 7:24
Binding is merged into default branch (see
http://code.google.com/p/openhab/source/detail?r=86aa9f32887c813c309567250f8dddc
edf89f486).
Thanks Pali for another cool contribution!
Original comment by teichsta
on 11 Aug 2013 at 11:04
I think there is an issue with the "ERR"-command (or the response to it).
16:28:05.973 DEBUG o.o.b.e.i.EpsonProjectorDevice[:358] - Query: 'ERR?'
16:28:06.075 DEBUG o.o.b.e.i.EpsonProjectorDevice[:361] - Response: 'ERR'
16:28:06.075 WARN o.o.b.e.i.EpsonProjectorBinding[:372] - Couldn't execute
command 'ErrCode',
org.openhab.binding.epsonprojector.internal.EpsonProjectorException: Error
response received
16:28:06.076 ERROR o.o.b.e.i.EpsonProjectorBinding[:213] - No response received
from command 'ErrCode'
Frankly, I'm not sure if the Err-commands is supported by my TW3600.
Original comment by christop...@gmail.com
on 17 Aug 2013 at 2:55
Since this issue is closed could you open another issue if there are issue with
the epson binding? Best, Thomas E.-E.
Original comment by teichsta
on 18 Aug 2013 at 7:02
https://code.google.com/p/openhab/issues/detail?id=406
Original comment by christop...@gmail.com
on 18 Aug 2013 at 10:39
Original issue reported on code.google.com by
pauli.an...@gmail.com
on 27 Apr 2013 at 9:45