Add sensors to Motionblinds BLE integration #114226

LennP · 2024-03-26T09:48:45Z

Proposed change

Add sensors to Motionblinds BLE integration (battery percentage, connection status, calibration status, signal strength).

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request: Add sensor platform to Motionblinds BLE home-assistant.io#32046

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

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

Documentation added/updated for www.home-assistant.io

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

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant · 2024-03-26T09:48:51Z

Hey there @jerrybboy, mind taking a look at this pull request as it has been labeled with an integration (motionblinds_ble) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of motionblinds_ble can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign motionblinds_ble Removes the current integration label and assignees on the pull request, add the integration domain after the command.
@home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
@home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

homeassistant/components/motionblinds_ble/sensor.py

joostlek · 2024-03-26T10:03:09Z

homeassistant/components/motionblinds_ble/strings.json

+      "connection": {
+        "name": "Connected",
+        "state": {
+          "connected": "Yes",
+          "disconnected": "No",
+          "connecting": "Connecting...",
+          "disconnecting": "Disconnecting..."
+        }
+      },
+      "calibration": {
+        "name": "Calibrated",
+        "state": {
+          "calibrated": "Yes",
+          "uncalibrated": "No",
+          "calibrating": "Calibrating..."
+        }
+      },


I think these entities could be more descriptive in their value.

Name: connectivity
connected
disconnected
connecting
disconnecting

Personally I think "Connectivity: Connected" is a bit too verbose. The majority of times, currently it will say "Connected: Yes" or "Connected: No" which I think is nicer. "Connecting" and "Disconnecting" are kind of transitioning states to provide the user with a bit more information on what is happening

That's why I opted for "Yes"/"No", I think it's easier to read

It might look good on a dashboard, but it's horrible for automating and templating

Don't you use the key for automation and not the translation? For the majority of users I personally think this is the best option. "Connected: Yes" in my opinion is much easier to process (and not to mention translate to other languages) than something like "Connectivity: Connected"

"state": { "connected": "Yes", "disconnected": "No", "connecting": "Connecting...", "disconnecting": "Disconnecting..." }

For this one I'm looking to the connectivity device class, which is used in the binary sensors and looks like connectivity - connected - disconnected. Even tho this provides own translations I still think they should be in line with the translations other integrations use

What about:

"connection": { "name": "Connection", "state": { "connected": "Connected", "disconnected": "Disconnected", "connecting": "Connecting", "disconnecting": "Disconnecting" } },

But as you know this may result in longer translations in other languages, where in Dutch you would probably get "Verbinding: Verbonden", as opposed to "Verbonden: Ja". I still prefer the latter but if you want the former I will change it

You could do Connection status

Fixed it, please check

I hope "Connection status: Disconnecting..." fits in the sensor entity?

requirements_all.txt

homeassistant/components/motionblinds_ble/sensor.py

home-assistant · 2024-03-26T10:04:14Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

homeassistant/components/motionblinds_ble/icons.json

homeassistant/components/motionblinds_ble/sensor.py

…alue

homeassistant/components/motionblinds_ble/strings.json

homeassistant/components/motionblinds_ble/sensor.py

homeassistant/components/motionblinds_ble/strings.json

joostlek

wrong button

Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

home-assistant bot added cla-signed integration: motionblinds_ble new-feature new-platform by-code-owner Quality Scale: No score labels Mar 26, 2024

LennP changed the title ~~Add sensors~~ Add sensors to Motionblinds BLE integration Mar 26, 2024

joostlek requested changes Mar 26, 2024

View reviewed changes

home-assistant bot marked this pull request as draft March 26, 2024 10:04

LennP requested a review from joostlek March 26, 2024 10:59

LennP marked this pull request as ready for review March 26, 2024 10:59

LennP force-pushed the motionblinds_ble_sensor_pr branch from 82bac04 to 5984401 Compare March 26, 2024 12:31

LennP mentioned this pull request Mar 26, 2024

Add sensor platform to Motionblinds BLE home-assistant/home-assistant.io#32046

Open

8 tasks

LennP force-pushed the motionblinds_ble_sensor_pr branch from 5984401 to 4d54c56 Compare March 27, 2024 10:32

edenhaus requested changes Mar 27, 2024

View reviewed changes

homeassistant/components/motionblinds_ble/icons.json Outdated Show resolved Hide resolved

homeassistant/components/motionblinds_ble/sensor.py Outdated Show resolved Hide resolved

home-assistant bot marked this pull request as draft March 27, 2024 16:41

LennP force-pushed the motionblinds_ble_sensor_pr branch from 24e30e7 to 5d070c7 Compare March 27, 2024 17:01

LennP requested a review from edenhaus March 27, 2024 17:10

LennP marked this pull request as ready for review March 27, 2024 17:10

LennP force-pushed the motionblinds_ble_sensor_pr branch 3 times, most recently from 8635ec5 to 2f8f3a3 Compare April 12, 2024 09:56

LennP added 6 commits April 12, 2024 11:45

Add sensors

9fbaf33

Add sensor.py to .coveragerc

96fb1c6

Move icons to icons.json

f0b4376

Remove signal strength translation key

7fa6218

Change native_value attribute name in entity description to initial_v…

7486d42

…alue

Use str instead of enum for MotionConnectionType for options

51893d1

LennP added 6 commits April 12, 2024 11:45

Add calibration options to entity description

fa1752f

Fix icons

2fcfc00

Change translations of connection and calibration

f7aba15

Move entity descriptions to __init__

8fa1978

Use generic sensor class

f15aab4

Use generic sensor class

bd0e763

LennP force-pushed the motionblinds_ble_sensor_pr branch from 2f8f3a3 to bd0e763 Compare April 12, 2024 11:45

joostlek approved these changes Jun 5, 2024

View reviewed changes

joostlek requested changes Jun 5, 2024

View reviewed changes

home-assistant bot marked this pull request as draft June 5, 2024 10:40

LennP and others added 4 commits June 6, 2024 10:40

Update homeassistant/components/motionblinds_ble/strings.json

11712b5

Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

Update homeassistant/components/motionblinds_ble/sensor.py

2444618

Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

Update homeassistant/components/motionblinds_ble/strings.json

aa76ab7

Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

Update homeassistant/components/motionblinds_ble/strings.json

816c03b

Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

LennP requested a review from joostlek June 6, 2024 08:41

LennP marked this pull request as ready for review June 6, 2024 08:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add sensors to Motionblinds BLE integration #114226

Add sensors to Motionblinds BLE integration #114226

LennP commented Mar 26, 2024 •

edited

home-assistant bot commented Mar 26, 2024

joostlek Mar 26, 2024

LennP Mar 26, 2024

LennP Mar 26, 2024

joostlek Mar 26, 2024

LennP Mar 26, 2024

joostlek Mar 27, 2024

LennP Mar 27, 2024

joostlek Mar 27, 2024

LennP Mar 27, 2024

LennP Mar 27, 2024

home-assistant bot commented Mar 26, 2024

joostlek left a comment

Add sensors to Motionblinds BLE integration #114226

Are you sure you want to change the base?

Add sensors to Motionblinds BLE integration #114226

Conversation

LennP commented Mar 26, 2024 • edited

Proposed change

Type of change

Additional information

Checklist

home-assistant bot commented Mar 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

home-assistant bot commented Mar 26, 2024

joostlek left a comment

Choose a reason for hiding this comment

LennP commented Mar 26, 2024 •

edited