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

Discovery Tweaks After Initial Usage #390

Merged
merged 11 commits into from
Apr 26, 2021

Conversation

krkeegan
Copy link
Collaborator

Proposed change

These are a few small fixes to the Discovery platform that I found while setting it up.

  1. HomeAssistant does not allow periods in the node_id or the object_id in the topic. They only allow a small set of characters. This does not carry through to things in the payload. Solved this by changing any out of bounds characters to underscore.
  2. The logic in loading the modem discovery topics was flawed. This fixes it.
  3. Some other small fixes to the suggested config.

Checklist

  • The code change is tested and works locally.
  • Local tests pass.
  • Tests have been added to verify that the new code works.
  • Code documentation was added where necessary

Squelches a bunch of potential discovery log messages that are
unnecessary.
Necessary because the modem only uses a single template with
an indeterminate, until run time, number of topics generated
The HA implementation of the discovery protocol only allows a
very small subset of characters for use in the node_id and
the object_id.  This replaces all un-allowed characters with an
underscore.

Also removed the ability to set the topic via yaml.  This never
really worked anyways and would create potential problems.

Reverted changing the periods in the address to underscores,
periods are still fine in the unique_id, just not in the topic
string.
@krkeegan krkeegan merged commit 02ced94 into TD22057:Discovery Apr 26, 2021
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.

None yet

1 participant