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

Merging LWT into HA discovery #1

Closed

Conversation

anastas78
Copy link
Owner

In my opinion LWT is concept really useful for automation systems -i.e. HomeAssistant or in other live data solutions - i.e. skin belchertown. Out of that scenario I don't see a reason for weewx-mqtt to set last will. @ThomDietrich do you agree?
However in HA it is a big plus to have it as option. I would suggest to make it an config option(Fixes 21) and set it to True if ha_discovery is true. Then we can have a device_tracker for weewx and availability status for the sensors.

If you agree I can merge the two PR in one

@anastas78 anastas78 closed this Oct 17, 2022
@anastas78 anastas78 reopened this Oct 17, 2022
@anastas78 anastas78 marked this pull request as ready for review October 17, 2022 18:36
@ThomDietrich
Copy link

ThomDietrich commented Oct 17, 2022

Generally agree.

What's your goal here?
If your aim is to provide a PR to Matthew, merging the two might be irritating
If your aim is to maintain an independent fork, I am all for it.

Would you like me to review your HA discovery implementation? Edit: Just saw your PR. I'll respond there.

Regarding the work on this project: I suggested to Matthew to found a GitHub organization under which we could maintain a bunch of weewx extensions as a bigger community. Would you be interested to discuss this idea?

@anastas78
Copy link
Owner Author

anastas78 commented Oct 17, 2022

What's your goal here?
If your aim is to provide a PR to Matthew, merging the two might be irritating
If your aim is to maintain an independent fork, I am all for it.

Merging the two PRs is just an option so we comment and work simultaneously and don't have to rework code later.
Edit:

Out of that scenario I don't see a reason for weewx-mqtt to set last will

That way LWT can be set/used only when ha_discovery is True.

Maintain an independent fork is a must at least till Matthew has the time to look at that extension again.

Regarding the work on this project: I suggested to Matthew to found a GitHub organization under which we could maintain a bunch of weewx extensions as a bigger community. Would you be interested to discuss this idea?

Sure. Just still learning how all of this work - i.e. that PR should have been targeted to a new branch ha-discovery-with-lwt and not the master, but still wondering how to rebase it :)

@anastas78 anastas78 changed the base branch from master to ha-discovery-with-lwt October 17, 2022 19:48
@anastas78
Copy link
Owner Author

I've merged your patch together with the HA discovery into a new branch ha-discovery-with-lwt and added the device tracker and availability options.

I will be delighted if you review it and comment on it.

@anastas78 anastas78 closed this Aug 18, 2023
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.

Last will and testament missing
2 participants