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

Fix support for base topic for empty values in MQTT discovery msg #19501

Merged
merged 2 commits into from Dec 24, 2018

Conversation

Projects
None yet
5 participants
@emontnemery
Copy link
Contributor

emontnemery commented Dec 21, 2018

Description:

Fix error when discovery message contains empty values and a base topic is defined.

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

@emontnemery emontnemery requested a review from home-assistant/core as a code owner Dec 21, 2018

@wafflebot wafflebot bot added the in progress label Dec 21, 2018

@emontnemery emontnemery changed the title Fix topic expansion for empty value MQTT discovery: Fix support for of base topic for empty values Dec 21, 2018

@fabaff fabaff changed the title MQTT discovery: Fix support for of base topic for empty values Fix support for of base topic for empty values of MQTT discovery Dec 23, 2018

@emontnemery emontnemery changed the title Fix support for of base topic for empty values of MQTT discovery Fix support for base topic for empty values in MQTT discovery msg Dec 23, 2018

@fabaff

fabaff approved these changes Dec 24, 2018

Copy link
Member

fabaff left a comment

🐦

@fabaff fabaff merged commit edb7ec7 into home-assistant:dev Dec 24, 2018

5 checks passed

Hound No violations found. Woof!
WIP Legacy commit status override — see details
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.008%) to 93.047%
Details

@wafflebot wafflebot bot removed the in progress label Dec 24, 2018

dshokouhi added a commit to dshokouhi/home-assistant that referenced this pull request Dec 25, 2018

mxworm added a commit to mxworm/home-assistant that referenced this pull request Dec 25, 2018

Merge branch 'dev' into current
* dev: (27 commits)
  Add device_id configuration option to Bluetooth tracker (home-assistant#18539)
  Add homematicip cloud full flush measuring switch (home-assistant#19247)
  Added support for triggered state on NX584 alarm. (home-assistant#19524)
  Add HomematicIP SMI55 device (home-assistant#19400)
  Clean up homematicip cloud (home-assistant#19481)
  Improve Lutron RadioRA2 support, adding switches and scenes (home-assistant#18330)
  Remove global from ZHA application controller (home-assistant#19557)
  Update homekit controller to homekit==0.12.0 (home-assistant#19549)
  Improve handling of MQTT light discovery (home-assistant#19436)
  Fix support for base topic for empty values in MQTT discovery msg (home-assistant#19501)
  Configure ZHA entity on new ZHA device join (home-assistant#19470)
  Add hub- and device-info for tellduslive (home-assistant#19180)
  Fix issues in ZHA light (home-assistant#19368)
  Updated to support per device find iphone sound. (home-assistant#19535)
  Change ISY binary_sensor subnode to hex (home-assistant#19471)
  increase robustness, when something upstream fails (home-assistant#19493)
  Support ZHA light turn_off transition (home-assistant#19531)
  Pywemo version bump (home-assistant#19538)
  Make ZHA entities non-polled by default (home-assistant#19536)
  Add ZHA occupancy sensor (home-assistant#19365)
  ...
@fsaris

This comment has been minimized.

Copy link

fsaris commented Jan 5, 2019

For the record (and others searching for this) this fixes the following error?

Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/homeassistant/components/mqtt/discovery.py", line 200, in async_device_message_received
    if value[0] == TOPIC_BASE and key.endswith('_topic'):
IndexError: string index out of range

Used auto discover in combination with Tasmota https://github.com/arendst/Sonoff-Tasmota/wiki/Home-Assistant

@emontnemery

This comment has been minimized.

Copy link
Contributor

emontnemery commented Jan 6, 2019

@fsaris Or of curiosity, when does Tasmota trigger this bug?

@fsaris

This comment has been minimized.

Copy link

fsaris commented Jan 7, 2019

For me when I set gpio 14 to switch 2n. And have a custom topic set. Base topic is then a empty string in the config payload.

@emontnemery

This comment has been minimized.

Copy link
Contributor

emontnemery commented Jan 7, 2019

Can you give an example of the resulting MQTT discovery message and how you configured Tasmota to send the custom topic?

@engrbm87

This comment has been minimized.

Copy link
Contributor

engrbm87 commented Jan 7, 2019

I created a pull request that solves this issue in tasmota code. this has been merged into the development branch

@fsaris

This comment has been minimized.

Copy link

fsaris commented Jan 7, 2019

Tnx, do you have a link of the pr?

@fsaris

This comment has been minimized.

Copy link

fsaris commented Jan 7, 2019

Just tested it and it works now with Tasmota again without this fix in HA

Tnx :)

@emontnemery

This comment has been minimized.

Copy link
Contributor

emontnemery commented Jan 7, 2019

@fsaris I'd still really like to understand how you configured Tasmota for a custom topic which resulted in an empty base topic.

@fsaris

This comment has been minimized.

Copy link

fsaris commented Jan 7, 2019

Didn't test without custom topic, so maybe the base topic is also empty when keeping it default.

I used command SwitchTopic to set the topic for my PIR binary sensor.

@engrbm87

This comment has been minimized.

Copy link
Contributor

engrbm87 commented Jan 8, 2019

@emontnemery If you choose a different mqtt topic with 'SwitchTopic' command the Prefix function that figures out the base topic will come up empty because the state topic and the availability topic will be different. so the '~' will end up empty.

@balloob balloob referenced this pull request Jan 10, 2019

Merged

0.85.0 #19897

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment