8000 Mysensors: Refactor s_types and v_types mapping by MartinHjelmare · Pull Request #1034 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Mysensors: Refactor s_types and v_types mapping #1034

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

Merged

Conversation

MartinHjelmare
Copy link
Member

To be able to handle new platforms with multiple used v_types but
single states, a new more strict mapping was required. Each s_type is
now mapped to its corresponding v_type(s) with a defaultdict(list).

@@ -125,7 +124,8 @@ def mysensors_callback(gateway, node_id):
for child in gateway.sensors[node_id].children.values():
for value_type in child.values.keys():
key = node_id, child.id, value_type
if child.type not in s_types or value_type not in v_types:
if child.type not in map_sv_types or \
Copy link
Member

Choose a reason for hiding this comment

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

You're using a defaultdict so there is no need to test if child.type in map_sv_types

Copy link
Member

Choose a reason for hiding this comment

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

Actually, not a single assumption in your code seems to use the fact that you're using a defaultdict.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I came to the same conclusion after a nights sleep. I'll change to a regular dict of lists. That way, I don't have to worry about mutating the map by accident, eg when checking membership.

@balloob
8000
Copy link
Member
balloob commented Jan 29, 2016

It looks fine 🐬 . Merge when you're happy about it.

To be able to handle new platforms with multiple used v_types but
single states, a new more strict mapping was required. Each s_type is
now mapped to its corresponding v_type(s) in a dict of lists.
@MartinHjelmare
Copy link
Member Author

Changed the defaultdict(list) for a regular dict of lists. Merging.

MartinHjelmare added a commit that referenced this pull request Jan 30, 2016
Mysensors: Refactor s_types and v_types mapping
@MartinHjelmare MartinHjelmare merged commit a8f7bc2 into home-assistant:dev Jan 30, 2016
@MartinHjelmare MartinHjelmare deleted the refactor-s_v_types branch January 30, 2016 00:04
@home-assistant home-assistant locked and limited conversation to collaborators Mar 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0