Add support for Bosch BMP280 Sensor by belidzs · Pull Request #30837 · home-assistant/core

belidzs · 2020-01-16T13:34:26Z

Description:

Implement support for the Bosch BMP280 Environmental Sensor

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#11769

Example entry for `configuration.yaml` (if applicable):

sensor:
  - platform: bmp280

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.
I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
Untested files have been added to .coveragerc.

If the code does not interact with devices:

Tests have been added to verify that the new code works.

homeassistant · 2020-01-16T13:34:29Z

Hi @belidzs,

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!

springstan · 2020-01-16T18:51:04Z

@belidzs please take a look at the pylint errors :)

belidzs · 2020-01-16T19:03:51Z

@springstan my bad, I thought running tox -e lint would also run pylint. It comes back without errors now.

belidzs · 2020-01-16T19:55:50Z

It looks like the build is failing because of an other component in the repo called rpi_gpio.

I have fetched the latest dev and rebased my branch on it to see if it's been fixed since, but the linter still fails:

root@88d1efd412a7:/workspaces/home-assistant# pylint homeassistant/components/rpi_gpio
************* Module homeassistant.components.rpi_gpio
homeassistant/components/rpi_gpio/__init__.py:18:8: E1101: Module 'RPi.GPIO' has no 'cleanup' member (no-member)
homeassistant/components/rpi_gpio/__init__.py:25:4: E1101: Module 'RPi.GPIO' has no 'setmode' member (no-member)
homeassistant/components/rpi_gpio/__init__.py:25:17: E1101: Module 'RPi.GPIO' has no 'BCM' member (no-member)
homeassistant/components/rpi_gpio/__init__.py:31:4: E1101: Module 'RPi.GPIO' has no 'setup' member (no-member)
homeassistant/components/rpi_gpio/__init__.py:31:21: E1101: Module 'RPi.GPIO' has no 'OUT' member (no-member)
homeassistant/components/rpi_gpio/__init__.py:36:4: E1101: Module 'RPi.GPIO' has no 'setup' member (no-member)
homeassistant/components/rpi_gpio/__init__.py:36:21: E1101: Module 'RPi.GPIO' has no 'IN' member (no-member)
homeassistant/components/rpi_gpio/__init__.py:36:30: E1101: Module 'RPi.GPIO' has no 'PUD_DOWN' member (no-member)
homeassistant/components/rpi_gpio/__init__.py:36:72: E1101: Module 'RPi.GPIO' has no 'PUD_UP' member (no-member)
homeassistant/components/rpi_gpio/__init__.py:41:4: E1101: Module 'RPi.GPIO' has no 'output' member (no-member)
homeassistant/components/rpi_gpio/__init__.py:46:11: E1101: Module 'RPi.GPIO' has no 'input' member (no-member)
homeassistant/components/rpi_gpio/__init__.py:51:4: E1101: Module 'RPi.GPIO' has no 'add_event_detect' member (no-member)
homeassistant/components/rpi_gpio/__init__.py:51:32: E1101: Module 'RPi.GPIO' has no 'BOTH' member (no-member)

fabaff · 2020-02-01T23:21:57Z

Why does this integration needs RPi.GPIO?

adafruit-circuitpython-bmp280 depends on Adafruit-Blinka while it conditionally depends on RPi.GPIO (on arm architecture anyway)

I wasn't sure if these conditional requirements will be automatically resolved by home-assistant but a tutorial written by the library author explicitly mentioned it as a requirement so I added it to the requirements as well.

As it is commented out by gen_requirements_all.py anyways I think it can be safely removed.

@fabaff is this blocking the merge?

belidzs · 2020-03-12T12:32:57Z

Is there anything I need to do at this point?

Co-Authored-By: springstan <46536646+springstan@users.noreply.github.com>

bdraco

This is getting close. Are the docs still accurate?

belidzs · 2020-03-21T15:49:27Z

@bdraco not really, I have to update it due to the fact that the i2c_address parameter became required and there's no default value provided anymore. I'll do that soon as well.

belidzs · 2020-03-21T17:26:17Z

Fixed docs as well at #11769

bdraco

LGTM

belidzs · 2020-03-21T22:06:22Z

@bdraco thanks for the review, I appreciate it

MartinHjelmare

Mostly small things. Please address the comments in a new PR.

* Fix minor issues mentioned in #30837 after it was closed * Update homeassistant/components/bmp280/sensor.py Co-Authored-By: Martin Hjelmare <marhje52@gmail.com> Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

homeassistant added the cla-needed label Jan 16, 2020

probot-home-assistant Bot added new-integration integration: bmp280 labels Jan 16, 2020

belidzs mentioned this pull request Jan 16, 2020

Added documentation for the bmp280 integration home-assistant/home-assistant.io#11769

Merged

2 tasks

homeassistant added cla-signed and removed cla-needed labels Jan 16, 2020

springstan changed the title ~~Implement support for Bosch BMP280 Sensor~~ Add support for Bosch BMP280 Sensor Jan 16, 2020

belidzs mentioned this pull request Jan 17, 2020

Make gen_requirements_all.py case insensitive for ignored packages #30885

Merged

9 tasks

fabaff reviewed Feb 1, 2020

View reviewed changes

springstan reviewed Feb 2, 2020

View reviewed changes

Comment thread homeassistant/components/bmp280/manifest.json Outdated

Comment thread homeassistant/components/bmp280/sensor.py Outdated

bdraco requested changes Mar 15, 2020

View reviewed changes

belidzs and others added 7 commits March 21, 2020 16:00

Implement support for Bosch BMP280 Sensor

99697ff

Fixed linting errors

25943b0

Fixed case of the requirement RPi.GPIO so it is ignored in requirements

157749d

Update homeassistant/components/bmp280/manifest.json

3bd54bf

Co-Authored-By: springstan <46536646+springstan@users.noreply.github.com>

Update homeassistant/components/bmp280/sensor.py

369120c

Co-Authored-By: springstan <46536646+springstan@users.noreply.github.com>

Fix linting errors

f05fd50

Implemented changes suggested by code review

6169c8c

belidzs requested a review from bdraco March 21, 2020 15:12

bdraco reviewed Mar 21, 2020

View reviewed changes

Comment thread homeassistant/components/bmp280/sensor.py

bdraco requested changes Mar 21, 2020

View reviewed changes

Fixed incomplete refactoring

c18b3ff

belidzs requested a review from bdraco March 21, 2020 17:25

bdraco approved these changes Mar 21, 2020

View reviewed changes

bdraco merged commit df67ab9 into home-assistant:dev Mar 23, 2020

MartinHjelmare reviewed Mar 23, 2020

View reviewed changes

Comment thread homeassistant/components/bmp280/sensor.py

Comment thread homeassistant/components/bmp280/sensor.py

Comment thread homeassistant/components/bmp280/sensor.py

Comment thread homeassistant/components/bmp280/sensor.py

belidzs deleted the bmp280 branch March 24, 2020 14:05

belidzs mentioned this pull request Mar 24, 2020

Fix minor bmp280 issues mentioned in late review #33211

Merged

20 tasks

lock Bot locked and limited conversation to collaborators Mar 27, 2020

Uh oh!

Conversation

belidzs commented Jan 16, 2020

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

homeassistant commented Jan 16, 2020

Uh oh!

springstan commented Jan 16, 2020

Uh oh!

belidzs commented Jan 16, 2020

Uh oh!

belidzs commented Jan 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fabaff Feb 1, 2020

Choose a reason for hiding this comment

Uh oh!

belidzs Feb 1, 2020

Choose a reason for hiding this comment

Uh oh!

belidzs Feb 8, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

belidzs commented Mar 12, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bdraco left a comment

Choose a reason for hiding this comment

Uh oh!

belidzs commented Mar 21, 2020

Uh oh!

belidzs commented Mar 21, 2020

Uh oh!

bdraco left a comment

Choose a reason for hiding this comment

Uh oh!

belidzs commented Mar 21, 2020

Uh oh!

MartinHjelmare left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Example entry for `configuration.yaml` (if applicable):

belidzs commented Jan 16, 2020 •

edited

Loading