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

Use serial_for_url for serial port, allowing rfc2217 #4

Merged
merged 2 commits into from
Jan 3, 2023

Conversation

xouillet
Copy link
Contributor

Hi,

As my the serial port connected to my Linky is connected to another host than the one running home-assistant, I nedded to use rfc2217 which is supported by pyserial, via the serial_for_url method (cf. https://pyserial.readthedocs.io/en/latest/url_handlers.html)

Moreover, serial_for_url can be used as a drop-in replacement for standard Serial constructor, that's what is done in this PR.

As an example, the home assistant yaml can now contains:

lixeeticdin:
  serial_port: rfc2217://192.168.1.254:10000?ign_set_control

@hekmon
Copy link
Owner

hekmon commented Jan 1, 2023

I was not aware of such possibility, thank you for your input !

Could I ask a few modifications before merging it ?

  • Rebase on the v2 branch as there have been a lot of modifications since (currently released as beta)
  • Update the readme to indicate the support of this new capability
  • Update the strings to better reflect this change (something like "serial device address" to keep it universal ? If you think of a better term, go with it. There is 3 locations for strings: main file, en generated translation and fr translation)

Once merged, I will release a 3rd beta with it :)

Thanks again !

@xouillet
Copy link
Contributor Author

xouillet commented Jan 2, 2023

Oh, I wasn't aware a v2 is on the way, nice !

I've made the changes, and tested it on my installation, with the new config flow, it works ok.

I took the liberty to add my device in the README in a separate commit but the same PR, I can create a separate PR for this if you want.

@hekmon
Copy link
Owner

hekmon commented Jan 3, 2023

No that's fine the way it is, do not worry. Still have to test it before tagging it and release the 3rd beta. Will do asap.

Thanks again !

@hekmon hekmon merged commit c5aed65 into hekmon:v2 Jan 3, 2023
@xouillet xouillet deleted the main branch January 5, 2023 15:34
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.

2 participants