Skip to content
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

feat(hass): use binary sensors when notifications has only two states #396

Merged
merged 19 commits into from
Feb 1, 2021

Conversation

varet80
Copy link
Collaborator

@varet80 varet80 commented Jan 30, 2021

Try to make Notifications better, byt creating HASS binary sensors.

  • cover as much as possible sensors
  • Make variables names better
  • Possibly improve flow
  • Support most sensors
  • Feedback

Fixes #374
Fixes #345
Fixes #118
Fixes #107

@blhoward2 Please review this change and Feedback is appreciated
If a device is not properly discovered please paste a Json payload on mqtt

@coveralls
Copy link

coveralls commented Jan 30, 2021

Pull Request Test Coverage Report for Build 527949988

  • 6 of 95 (6.32%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.09%) to 21.419%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/Gateway.js 6 95 6.32%
Files with Coverage Reduction New Missed Lines %
lib/Gateway.js 1 20.25%
Totals Coverage Status
Change from base Build 527902548: -0.09%
Covered Lines: 2049
Relevant Lines: 9795

💛 - Coveralls

Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

Looks good @billiaz ! :)

lib/Gateway.js Outdated Show resolved Hide resolved
@varet80 varet80 changed the title [feat] Publish binary sensors when notifications has only two states feat: Publish binary sensors when notifications has only two states Jan 31, 2021
@varet80 varet80 changed the title feat: Publish binary sensors when notifications has only two states feat: publish binary sensors when notifications has only two states Jan 31, 2021
@blhoward2
Copy link
Collaborator

blhoward2 commented Jan 31, 2021

Ok, I've tested this. I see the new binary sensors under the zwavejs2mqtt front-end for the node (130 in this case, an Aeotec Water Sensor 6), but they never appears in HA. I even tried rediscovering the node and multiple restarts of HA. You'll see more than one startup in the js log because I realized I had mqtt logging turned off. You'll also see that I had to wake the water sensor a few times but ultimately it went to interview complete.

zwavejs_1.log
zwavejs2mqtt.log.zip
node_130.json.zip

Copy link
Collaborator Author

@varet80 varet80 left a comment

Choose a reason for hiding this comment

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

Ok, I've tested this. I see the new binary sensors under the zwavejs2mqtt front-end for the node (130 in this case, an Aeotec Water Sensor 6), but they never appears in HA. I even tried rediscovering the node and multiple restarts of HA. You'll see more than one startup in the js log because I realized I had mqtt logging turned off. You'll also see that I had to wake the water sensor a few times but ultimately it went to interview complete.

zwavejs_1.log
zwavejs2mqtt.log.zip
node_130.json.zip

I do see the binary sensors are generated for HASS.
I suspect something breaks in hass. to get an entity error frm HASS is good to have DEBUG on (been there!)

        "binary_sensor_cover_status": {
            "type": "binary_sensor",
            "object_id": "cover_status",
            "discovery_payload": {
                "payload_on": 3,
                "payload_off": 0,
                "value_template": "{{ value_json.value }}",
                "device_class": "opening",
                "state_topic": "homeassistant/130/113/0/Home_Security/Cover_status",
                "json_attributes_topic": "homeassistant/130/113/0/Home_Security/Cover_status",
                "device": {
                    "identifiers": [
                        "zwavejs2mqtt_0xeb87ad4a_node130"
                    ],
                    "manufacturer": "AEON Labs",
                    "model": "Water Sensor 6 (ZW122)",
                    "name": "nodeID_130",
                    "sw_version": "1.6"
                },
                "name": "nodeID_130_cover_status",
                "unique_id": "zwavejs2mqtt_0xeb87ad4a_130-113-0-Home_Security-Cover_status"
            },
            "discoveryTopic": "binary_sensor/nodeID_130/cover_status/config",
            "values": [
                "113-0-Home Security-Cover status"
            ],
            "persistent": false,
            "ignoreDiscovery": false
        },

another:

        "binary_sensor_water_alarm_1": {
            "type": "binary_sensor",
            "object_id": "water_alarm_1",
            "discovery_payload": {
                "payload_on": 2,
                "payload_off": 0,
                "value_template": "{{ value_json.value }}",
                "device_class": "None",
                "state_topic": "homeassistant/130/113/1/Water_Alarm/Sensor_status",
                "json_attributes_topic": "homeassistant/130/113/1/Water_Alarm/Sensor_status",
                "device": {
                    "identifiers": [
                        "zwavejs2mqtt_0xeb87ad4a_node130"
                    ],
                    "manufacturer": "AEON Labs",
                    "model": "Water Sensor 6 (ZW122)",
                    "name": "nodeID_130",
                    "sw_version": "1.6"
                },
                "name": "nodeID_130_water_alarm_1",
                "unique_id": "zwavejs2mqtt_0xeb87ad4a_130-113-1-Water_Alarm-Sensor_status"
            },
            "discoveryTopic": "binary_sensor/nodeID_130/water_alarm_1/config",
            "values": [
                "113-1-Water Alarm-Sensor status"
            ],
            "persistent": false,
            "ignoreDiscovery": false
        },
        "binary_sensor_water_alarm_2": {
            "type": "binary_sensor",
            "object_id": "water_alarm_2",
            "discovery_payload": {
                "payload_on": 2,
                "payload_off": 0,
                "value_template": "{{ value_json.value }}",
                "device_class": "None",
                "state_topic": "homeassistant/130/113/2/Water_Alarm/Sensor_status",
                "json_attributes_topic": "homeassistant/130/113/2/Water_Alarm/Sensor_status",
                "device": {
                    "identifiers": [
                        "zwavejs2mqtt_0xeb87ad4a_node130"
                    ],
                    "manufacturer": "AEON Labs",
                    "model": "Water Sensor 6 (ZW122)",
                    "name": "nodeID_130",
                    "sw_version": "1.6"
                },
                "name": "nodeID_130_water_alarm_2",
                "unique_id": "zwavejs2mqtt_0xeb87ad4a_130-113-2-Water_Alarm-Sensor_status"
            },
            "discoveryTopic": "binary_sensor/nodeID_130/water_alarm_2/config",
            "values": [
                "113-2-Water Alarm-Sensor status"
            ],
            "persistent": false,
            "ignoreDiscovery": false
        }
    },

i notice they have class as None! I will tryt o fit a class to make them look nicer.

from this link https://www.home-assistant.io/integrations/binary_sensor/#device-class

I suggest to use "moisture" or "problem" class.
I tend to believe moisture might fit better.

lib/Gateway.js Outdated Show resolved Hide resolved
lib/Gateway.js Outdated Show resolved Hide resolved
lib/Gateway.js Outdated Show resolved Hide resolved
lib/Gateway.js Outdated Show resolved Hide resolved
lib/Gateway.js Outdated Show resolved Hide resolved
lib/Gateway.js Outdated Show resolved Hide resolved
lib/Gateway.js Outdated Show resolved Hide resolved
robertsLando
robertsLando previously approved these changes Feb 1, 2021
Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

LGTM. @billiaz Feedback? Is this working?

@varet80
Copy link
Collaborator Author

varet80 commented Feb 1, 2021

I got some nice additions from @blhoward2 as two sensors are discovered but as Generic! and might worth include them!
that all! other than this for me it worked like charm!

@blhoward2 has a separate issue, which is causing his HASS not to discover

@blhoward2
Copy link
Collaborator

I'll try again tonight, my time. MQTT discovery has been working fine in other contexts so I'm not sure what the issue would be.

@robertsLando robertsLando changed the title feat: publish binary sensors when notifications has only two states feat(hass): use binary sensors when notifications has only two states Feb 1, 2021
@robertsLando
Copy link
Member

@billiaz This looks good to me, so if @blhoward2 feedback is ok I will merge this or let you do that :)

@aretakisv
Copy link
Contributor

I am adding a definition for notification device_classes to cover waterleak
i will put these under Moisture device_class

@aretakisv
Copy link
Contributor

@robertsLando it is ready!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment