Skip to content

Fixed the Wind sensor following new release of netatmo-api-python#8030

Merged
balloob merged 2 commits into
home-assistant:devfrom
glpatcern:master
Jun 16, 2017
Merged

Fixed the Wind sensor following new release of netatmo-api-python#8030
balloob merged 2 commits into
home-assistant:devfrom
glpatcern:master

Conversation

@glpatcern
Copy link
Copy Markdown
Contributor

Description:

The Wind NetAtmo sensor was unsupported and effectively broken so far.
Essentially, this commit adds a protection when adding incorrect monitored conditions to avoid to fail the entire NetAtmo component. Moreover for consistency reasons all conditions are renamed to lower case.

The real fix comes with python-netatmo-api 0.9.2, merged PR at:
jabesq/netatmo-api-python#5

No user configuration is changed by this patch. I did not run any integration test yet, I guess this will come by the PR itself. Obviously, "it works for me", but I'm available to execute any test as required.

The NetAtmo PR was at:
jabesq/netatmo-api-python#5

Essentially, this commit adds a protection when adding an incorrect
monitored conditions to avoid to fail the entire NetAtmo component,
plus for consistency reasons all conditions are now in lower case.
@homeassistant
Copy link
Copy Markdown
Contributor

Hi @glpatcern,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@homeassistant homeassistant added cla-needed integration: netatmo merging-to-master This PR is merging into the master branch and should probably change the branch to `dev`. platform: sensor.netatmo labels Jun 14, 2017
@mention-bot
Copy link
Copy Markdown

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

if variable in SENSOR_TYPES.keys():
dev.append(NetAtmoSensor(data, module_name, variable))
else:
_LOGGER.warning("Ignoring unknown var %s for mod %s", \
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the backslash is redundant between brackets

@balloob balloob changed the base branch from master to dev June 15, 2017 05:39
@balloob
Copy link
Copy Markdown
Member

balloob commented Jun 15, 2017

You deleted the instruction to run script/gen_requirements.py… please do so anyway to get CI to pass.

@pvizeli pvizeli removed the merging-to-master This PR is merging into the master branch and should probably change the branch to `dev`. label Jun 15, 2017
@glpatcern
Copy link
Copy Markdown
Contributor Author

Hi Paulus, sure - I thought the other instructions were not relevant but anyway I applied the required changes to hopefully get CI to pass.

@glpatcern
Copy link
Copy Markdown
Contributor Author

...which now fails on a different unrelated place, where it was passing before. As far as I can see, the PR is clear, but please advice.

@balloob balloob merged commit 09ca440 into home-assistant:dev Jun 16, 2017
@balloob
Copy link
Copy Markdown
Member

balloob commented Jun 16, 2017

Awesome, thanks 👍

@balloob balloob mentioned this pull request Jun 16, 2017
@glpatcern
Copy link
Copy Markdown
Contributor Author

You're welcome! Glad to have contributed to this project, and I expect some more contributions in the near future...

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.

7 participants