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

Some commands not working on Serial (and probably TCP) and questions about the package #25

Open
mvdwetering opened this issue Jan 13, 2025 · 0 comments

Comments

@mvdwetering
Copy link

I noticed that selecting source from Home Assistant does not work for my projector (EH-TW3200). I had been using the integration for a while, but only turned on/off the projector which works fine.

Looking at the debug logs it shows the command "HDMI2" is sent and not "KEY 40" which I think is intended.

2025-01-11 23:09:47.042 DEBUG (MainThread) [epson_projector.projector] Sending command to projector HDMI2
2025-01-11 23:09:47.042 DEBUG (MainThread) [epson_projector.projector_serial] Sent to Epson: 'HDMI2\r' with timeout 3
2025-01-11 23:09:47.087 DEBUG (MainThread) [epson_projector.projector_serial] Response from Epson 'ERR'
2025-01-11 23:09:47.088 ERROR (MainThread) [epson_projector.projector_serial] Error response to request 'HDMI2\r'

I see in the code that in projector_http.py the "command" is looked up in EPSON_KEY_COMMANDS and the corresponding value is sent. For projector_serial.py this is not the case, it just sends the "command" key.

After this I also realized the same problem exists for the get_property implementation, but because the key in EPSON_KEY_COMMANDS is the command that actually needs to be sent it happens to work for serial (except for VOLUME which should be the VOL on the serial line, but my projector does not have volume so never noticed it)

I wanted to do the same lookup for the get_property in serial, but then noticed that the commands used for get have "jsoncallback" in the values list which will not work for serial, it only should send the "COMMAND?" part.

    "PWR": [("jsoncallback", "PWR?")],
    "SOURCE": [("jsoncallback", "SOURCE?")],
    "CMODE": [("jsoncallback", "CMODE?")],
    "VOLUME": [("jsoncallback", "VOL?")],

I think the "jsoncallback" should not be in there and I am not even sure if the lookups are needed because the EPSON_KEY_COMMANDS seems to be mostly(?) for key codes.
I had a look at moving adding of jsoncallback to the projector_http.py file, but I could not find a description on what the final URL should look like. A link to the specs would be nice to get more understanding.

--- sorry for the wall of text below, but did not know where to put the analysis/question otherwise

While looking at this I started wondering what the intended design/interface of the package is.

The projector.get_pwr() and projector.get_serial() are clear.

The methods get_property() and send_command() are less clear to me.
The parameter is called "command", but it is not the ESC/VP21 command. It seems it needs to be a key from the EPSON_KEY_COMMANDS dict. But it is unclear to me why that that indirection is needed. There also seem to be duplicates like PWR_ON and TURN_ON.

I would like to propose to build an (in my opinion) easier to use interface on the package next to the existing one (so we don't break everything immediately).

The general idea would basically be get and set methods for every command on the projector class. These would hide the protocol details and use standard Python types so you don't need to parse the text response in Home Assistant.

E.g. projector.get_vol() would return an number or error (need to think more about error handing)
projector.set_key(keyname) where keyname would be an enum with supported key codes.
etc...

The get and set methods would then call functions on the projector transports similar to what get_property() and send_command() do now but without the current lookup. This would obviously need to be worked out a bit more, but this is the gist of it.

I would be willing to build this and migrate the Home Assistant integration to such a new interface. I would also want to bump minimum Python version to something newer e.g. 3.12 and add typehints and tests to the new code.

But I only have a serial projector to test against, not an http one which makes things challenging especially since I don't have a protocol specification for that protocol. I already built a simple simulator for a serial projector, a simple simulator for http would be convenient, but would need to test against real devices at some point.

Before I start on something like this I would also need to know if this is something that would be accepted as a PR (well, it will be multiple PRs otherwise it would be too big). Would like to hear your thoughts.

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

No branches or pull requests

1 participant