New amcrest binary sensor to monitor doorbell button #66302
Conversation
probot-home-assistant
bot
added
integration: amcrest
new-feature
small-pr
|
Hey there @flacjacket, mind taking a look at this pull request as it has been labeled with an integration ( |
MartinHjelmare
changed the title
| AmcrestSensorEntityDescription( | ||
| key=_DOORBELL_KEY, | ||
| name=_DOORBELL_NAME, | ||
| device_class=BinarySensorDeviceClass.OCCUPANCY, | ||
| event_code=_DOORBELL_EVENT_CODE, | ||
| ), |
There was a problem hiding this comment.
This PR violated ADR-0007 and ADR-0010
- https://github.com/home-assistant/architecture/blob/master/adr/0007-integration-config-yaml-structure.md
- https://github.com/home-assistant/architecture/blob/master/adr/0010-integration-configuration.md
This integration still uses a platform configuration and adding a sensor here affects YAML options for this integration. Additionally, it probably violates ADR-0003 as well, as this could be considered a monitored condition (which is also no longer allowed).
I suggest reverting this PR @Kane610 @PanicRide @flacjacket, as we no longer allow integrations to add or change a platform YAML configuration.
There was a problem hiding this comment.
Dang it! So much for helping with the review effort, sorry... I'll revert it tonight
There was a problem hiding this comment.
Aww... so much for finding something simple for my first contribution.
Don't worry, I still love you @frenck! :)
There was a problem hiding this comment.
Well, it's not personal @PanicRide! I definitely am thankful for the willingness to contribute! 鉂わ笍
There was a problem hiding this comment.
After reading those ADR documents, I agree with the concepts. I just wasn't aware of them. I don't take it personally. :)
Proposed change
This integration can now use a binary sensor to monitor the state of a built-in doorbell button found on some Amcrest camera devices.
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: