Skip to content

Tests for zwave workaround detection#6761

Merged
balloob merged 4 commits into
home-assistant:devfrom
emlove:zwave-workaround-tests
Mar 24, 2017
Merged

Tests for zwave workaround detection#6761
balloob merged 4 commits into
home-assistant:devfrom
emlove:zwave-workaround-tests

Conversation

@emlove
Copy link
Copy Markdown
Contributor

@emlove emlove commented Mar 23, 2017

Description:

See #6437

@mention-bot
Copy link
Copy Markdown

@armills, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @robbiet480 to be potential reviewers.

def get_device_component_mapping(value):
"""Get mapping of value to another component."""
if (value.node.manufacturer_id.strip() and
value.node.product_type.strip()):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this was ever actually doing anything. strip() returns a stripped copy of the string. Since there's always a string object returned it always evaluates to true. It doesn't modify the original string.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty string is considered false:

image

But I don't know if manufacturer_id or product_type would ever return a string that is just whitespace.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I must have messed something up when I tested it in the interpreter. I'll restore it then and add a test.

@balloob balloob merged commit cffc6c7 into home-assistant:dev Mar 24, 2017
@emlove emlove deleted the zwave-workaround-tests branch March 24, 2017 23:13
@fabaff fabaff mentioned this pull request Apr 6, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Jun 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants