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.

Sign up for GitHub

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

add support for generic zigbee sensor #1921

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+820 −0
Open

add support for generic zigbee sensor #1921

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7a4d856
add support for generic zigbee sensor
pInksenberg Feb 11, 2025
6b7d791
Replace batteryLevel with battery
FrankSpringfield Feb 19, 2025
fa1311c
Add test cases for added, receiving zonetype and profile changing
FrankSpringfield Feb 21, 2025
fb68377
improve code and add TC
pInksenberg Feb 21, 2025
f4cc05a
add configurations and related test cases
FrankSpringfield Feb 24, 2025
1dd4216
Add IasZoneType from lua.api
hongye-samsung Feb 25, 2025
6113590
Modify test case for changed drivers
FrankSpringfield Feb 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions drivers/SmartThings/zigbee-sensor/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
name: 'Zigbee Sensor'
packageKey: 'zigbee-sensor'
permissions:
zigbee: {}
description: "SmartThings driver for Zigbee sensor devices"
vendorSupportInformation: "https://support.smartthings.com"
8 changes: 8 additions & 0 deletions drivers/SmartThings/zigbee-sensor/fingerprints.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
zigbeeGeneric:
- id: "generic-sensor"
deviceLabel: "Zigbee Generic Sensor"
clusters:
server:
- 0x0500
deviceProfileName: generic-sensor

14 changes: 14 additions & 0 deletions drivers/SmartThings/zigbee-sensor/profiles/generic-contact-sensor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
name: generic-contact-sensor
components:
- id: main
capabilities:
- id: batteryLevel
version: 1
- id: contactSensor
version: 1
- id: firmwareUpdate
version: 1
- id: refresh
version: 1
categories:
- name: ContactSensor
14 changes: 14 additions & 0 deletions drivers/SmartThings/zigbee-sensor/profiles/generic-motion-sensor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
name: generic-motion-sensor
components:
- id: main
capabilities:
- id: batteryLevel
version: 1
- id: motionSensor
version: 1
- id: firmwareUpdate
version: 1
- id: refresh
version: 1
categories:
- name: MotionSensor
8 changes: 8 additions & 0 deletions drivers/SmartThings/zigbee-sensor/profiles/generic-sensor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
name: generic-sensor
components:
- id: main
capabilities:
- id: firmwareUpdate
version: 1
- id: refresh
version: 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I expect a category is required, but if that's not the case then this is fine.

Copy link
Contributor

@FrankSpringfield FrankSpringfield Feb 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This profile applies when the ZoneType of the IASZone has not yet been identified. In this case, the profile will be updated to other profiles to reflect other categories such as contact, motion, etc.
Until the device receives the ZoneType, we have no definitive information regarding its category; all we know is that it is a sensor.
Is there a specific device type that can solely describe a sensor?

Copy link
Contributor

Choose a reason for hiding this comment

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

We have a GenericSensor category I believe. Not positive what the corresponding icon is, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

We do found that device type in OCF device type wiki.

However, we have tested the profile with GenericSenso but noticed that there is no related icon displayed on the device card, only a default icon.
Upon investigation, we confirmed that the oic.d.sensor, which is the OCFdtype for GenericSensor, does not exist in the CN production environment.

Could you please assist us in confirming whether this device type exists in the Global production environment? If it does, we would like to request our cloud team to synchronize the device type with the CN production. Thank you for your assistance in advance.

Copy link
Contributor

Choose a reason for hiding this comment

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

It could be that the default icon is also what's used on the global environments. I'm not sure, though. It's not a new category, so I'd be surprised if it was different on CN.

Mainly this comment was just in case the driver packager complained about the profile not including a category. I thought it might have been a required field. If it's not a required field then you can ignore this comment. Default icon is fine for a device with undetermined functionality.

14 changes: 14 additions & 0 deletions drivers/SmartThings/zigbee-sensor/profiles/generic-waterleak-sensor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
name: generic-waterleak-sensor
components:
- id: main
capabilities:
- id: batteryLevel
version: 1
- id: waterSensor
version: 1
- id: firmwareUpdate
version: 1
- id: refresh
version: 1
categories:
- name: LeakSensor
157 changes: 157 additions & 0 deletions drivers/SmartThings/zigbee-sensor/src/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
-- Copyright 2025 SmartThings
--
-- Licensed under the Apache License, Version 2.0 (the "License");
-- you may not use this file except in compliance with the License.
-- You may obtain a copy of the License at
--
-- http://www.apache.org/licenses/LICENSE-2.0
--
-- Unless required by applicable law or agreed to in writing, software
-- distributed under the License is distributed on an "AS IS" BASIS,
-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-- See the License for the specific language governing permissions and
-- limitations under the License.

local ZigbeeDriver = require "st.zigbee"
local defaults = require "st.zigbee.defaults"
local clusters = require "st.zigbee.zcl.clusters"
local IASZone = clusters.IASZone
local capabilities = require "st.capabilities"
local ZONETYPE = "ZoneType"
local constants = require "st.zigbee.constants"
local PowerConfiguration = clusters.PowerConfiguration
local device_management = require "st.zigbee.device_management"

local CONTACT_SWITCH = 0x0015
Copy link
Contributor

Choose a reason for hiding this comment

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

You could use https://github.ecodesamsung.com/iot-hub/scripting-engine/blob/main/lua_libs/st/zigbee/generated/types/IasZoneType.lua instead of re-defining these here

Copy link

Choose a reason for hiding this comment

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

OK. we will modify it.

local MOTION_SENSOR = 0x000D
local WATER_SENSOR = 0x002A

local ZIGBEE_GENERIC_SENSOR_PROFILE = "generic-sensor"
local ZIGBEE_GENERIC_CONTACT_SENSOR_PROFILE = "generic-contact-sensor"
local ZIGBEE_GENERIC_MOTION_SENSOR_PROFILE = "generic-motion-sensor"
local ZIGBEE_GENERIC_WATERLEAK_SENSOR_PROFILE = "generic-waterleak-sensor"

-- ask device to upload its zone type
local ias_device_added = function(driver, device)
device:send(IASZone.attributes.ZoneType:read(device))
end

-- ask device to upload its zone status, then the status of capabilities can be synchronized
local ias_info_changed = function(driver, device)
Copy link
Contributor

Choose a reason for hiding this comment

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

This function will be run every time the device is updated on the cloud side. You should not be reading an attribute here. If the intent is for this to happen at some point after the device is added, you should use the call_with_delay to have the driver wait some period and then send the read.

device:send(IASZone.attributes.ZoneStatus:read(device))
end

-- update profile with different zone type
local function update_profile(device, zone_type)
local profile = ZIGBEE_GENERIC_SENSOR_PROFILE
if zone_type == CONTACT_SWITCH then
profile = ZIGBEE_GENERIC_CONTACT_SENSOR_PROFILE
elseif zone_type == MOTION_SENSOR then
profile = ZIGBEE_GENERIC_MOTION_SENSOR_PROFILE
elseif zone_type == WATER_SENSOR then
profile = ZIGBEE_GENERIC_WATERLEAK_SENSOR_PROFILE
end

device:try_update_metadata({profile = profile})
end

-- read zone type and update profile
local ias_zone_type_attr_handler = function (driver, device, attr_val)
device:set_field(ZONETYPE, attr_val.value)
update_profile(device, attr_val.value)
end

-- since we don't have button devices using IASZone, the driver here is remaining to be updated
local generate_event_from_zone_status = function(driver, device, zone_status, zb_rx)
Copy link
Contributor

@varzac varzac Feb 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Instead of having a single function here that that branches, the standard way to handle this in drivers is with subdrivers. So if you had a subdriver that looked like:

local capabilities = require "st.capabilities"
local clusters = require "st.zigbee.zcl.clusters"
local IASZone = clusters.IASZone

local subdriver_template

local generate_event_from_zone_status = function(driver, device, zone_status, zb_rx)
  if zone_status:is_alarm1_set() then
    device:emit_event(capabilities.contactSensor.contact.open())
  else
    device:emit_event(capabilities.contactSensor.contact.closed())
  end
end

local function subdriver_canhandle(opts, driver, device)
    -- Create a  "fields.lua" to contain this string definition and contact switch constant
    return device:get_field("ZoneType") == 0x0015, subdriver_template
end

-- These functions could be moved to a common file as well.
local ias_zone_status_attr_handler = function(driver, device, zone_status, zb_rx)
  generate_event_from_zone_status(driver, device, zone_status, zb_rx)
end

local ias_zone_status_change_handler = function(driver, device, zb_rx)
  generate_event_from_zone_status(driver, device, zb_rx.body.zcl_body.zone_status, zb_rx)
end

subdriver_template = {
    NAME = "Zigbee Contact",
    zigbee_handlers = {
      attr = {
        [IASZone.ID] = {
          [IASZone.attributes.ZoneStatus.ID] = ias_zone_status_attr_handler
        }
      },
      cluster = {
        [IASZone.ID] = {
          [IASZone.client.commands.ZoneStatusChangeNotification.ID] = ias_zone_status_change_handler
        }
      }
    },
    can_handle = subdriver_canhandle
  }
  
  return subdriver_template

There will be a bit more duplicate code, but the it allows us to separate all the different types into their own files, so if we need more complicated logic we don't end up with a big mess of a function.

Copy link
Contributor

Choose a reason for hiding this comment

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

@pInksenberg
For this, we need to check the ZoneType can be determined before the 'can_handle' triggered.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In this driver, we can seperate sub driver following zonetype, but this edge driver read zone type in add_handler. so this device wouldn't likely to handle some message in initial time potentially.

Copy link
Contributor

Choose a reason for hiding this comment

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

The same problem exists whether this is a subdriver or a main driver here, until the ZONETYPE field is set on the device, we don't know what type of event we can generate, so we can't generate any event when the value changes. I still prefer the subdriver format as I think it's easier to separate the different devices, but I don't feel strongly about it, and I would be OK with this initial version, but that if any changes are made to add more complex logic that we split things out into subdrivers.

local type = device:get_field(ZONETYPE)
local event
if type == CONTACT_SWITCH then
if zone_status:is_alarm1_set() then
Copy link
Contributor

Choose a reason for hiding this comment

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

The ZCL spec says that contact switches can use both alarm1 and alarm2 so I think both should be checked

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please check the newly commit

event = capabilities.contactSensor.contact.open()
else
event = capabilities.contactSensor.contact.closed()
end
elseif type == MOTION_SENSOR then
if zone_status:is_alarm1_set() then
Copy link
Contributor

Choose a reason for hiding this comment

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

Motion sensors can use either alarm1 or alarm2 so I would suggest reporting active if either are set

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please check the newly commit

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for making the changes, but can you use the or operator instead of the extra elseif because it's more concise/easier to read?

if zone_status:is_alarm1_set() or zone_status:is_alarm2_set() then
    event = capabilities.motionSensor.motion.active()
else
   event = capabilities.motionSensor.motion.inactive()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

modified

event = capabilities.motionSensor.motion.active()
else
event = capabilities.motionSensor.motion.inactive()
end
elseif type == WATER_SENSOR then
if zone_status:is_alarm1_set() then
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason why alarm2 isn't used as a trigger here?

Copy link
Contributor

Choose a reason for hiding this comment

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

if it should be, this function could be simplified to use a map

local event_mapping = {
  CONTACT_SWITCH = {active = capabilitiles.contactSensor.contact.open(), inactive = capabilities.contactSensor.contact.closed()}
  ...
}

if alarm1 or alarm2 then
  device:emit_event_for_endpoint(event_mapping[type].active)
else
  ...

something like that

Copy link

Choose a reason for hiding this comment

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

We will check that

Copy link
Contributor

@FrankSpringfield FrankSpringfield Feb 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

As we confirmed in ZCL library, the water sensor does not support alarm2 trigger.
(Table 8-5 in ZCL library file).

Value Zone Type Alarm1 Alarm2
0x0000 Standard CIE System Alarm -
0x000d Motion sensor Intrusion indication Presence indication
0x0015 Contact switch 1st portal Open-Close 2nd portal Open-Close
0x0016 Door/Window handle See Table 8-7 See Table 8-7
0x0028 Fire sensor Fire indication -
0x002a Water sensor Water overflow indication -

event = capabilities.waterSensor.water.wet()
else
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
else
else

nit: the linter noticed trailing whitespace here

event = capabilities.waterSensor.water.dry()
end
end
if event ~= nil then
device:emit_event_for_endpoint(
zb_rx.address_header.src_endpoint.value,
event)
if device:get_component_id_for_endpoint(zb_rx.address_header.src_endpoint.value) ~= "main" then
device:emit_event(event)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this being done to future-proof a possible implementation for a sensor with multiple different endpoints? I get the instinct but you'd have to have all the capabilities on the main component already in order to do this. It's probably best to just emit from the specific component rather than trying to (in the future) duplicate everything onto main as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, we will modify this one.

end
end

local ias_zone_status_attr_handler = function(driver, device, zone_status, zb_rx)
generate_event_from_zone_status(driver, device, zone_status, zb_rx)
end

local ias_zone_status_change_handler = function(driver, device, zb_rx)
generate_event_from_zone_status(driver, device, zb_rx.body.zcl_body.zone_status, zb_rx)
end

local battery_level_handler = function(driver, device, value, zb_rx)
local voltage = value.value
if voltage <= 25 then
Copy link
Contributor

Choose a reason for hiding this comment

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

It's pretty difficult to accurately translate voltage to battery level without knowing the details of the device and battery technology. I'd either suggest using the low battery indicator in the IAS Zone Status and/or switching from batteryLevel capability to battery and looking at the BatteryPercentageRemaining attribute instead of BatteryVoltage

Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies, I just saw the similar comment here: #1921 (comment)

Copy link

Choose a reason for hiding this comment

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

test result for BatteryPercentageRemaining:
using same type of battery
37 Meian meian water leak sensor Water Leak Sensor :100
20 EZVIZ EZVIZ T1C Smart Motion Sensor Motion sensor :90
21 Tuya Tuya Zigbee PIR Sensor Motion sensor :100
24 Tuya PIR Motion SensoR XZ-CGV2 Motion sensor :100
27 Tuya Tuya Human motion sensors Motion sensor :98

using different type of battery
38 EZVIZ EZVIZ T10C Water Leak Sensor Water Leak Sensor :98
34 Tuya Door Window Sensor Door and Window Sensor :100
33 EZVIZ EZVIZ T2C Door and Window Contact Sensor Door and Window Sensor :100
22 Tuya Tuya PIR Motion Sensor Motion sensor :100

device:emit_event(capabilities.batteryLevel.battery.critical())
elseif voltage < 28 then
device:emit_event(capabilities.batteryLevel.battery.warning())
else
device:emit_event(capabilities.batteryLevel.battery.normal())
end
end

local configure_handler = function(self, device)
device:send(device_management.build_bind_request(device, PowerConfiguration.ID, self.environment_info.hub_zigbee_eui))
device:send(PowerConfiguration.attributes.BatteryVoltage:configure_reporting(device, 30, 21600, 1))
device:send(PowerConfiguration.attributes.BatteryVoltage:read(device))
end

local zigbee_generic_sensor_template = {
supported_capabilities = {
capabilities.batteryLevel,
Copy link
Contributor

@inasail inasail Feb 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

You can delete this.
capabilities.batteryLevel,

Copy link

Choose a reason for hiding this comment

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

Ok, i will delete it

capabilities.firmwareUpdate,
capabilities.refresh
-- capabilities.motionSensor,
-- capabilities.contactSensor,
-- capabilities.waterSensor
Copy link
Contributor

Choose a reason for hiding this comment

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

Only the motionSensor here can work.

},
zigbee_handlers = {
attr = {
[IASZone.ID] = {
[IASZone.attributes.ZoneType.ID] = ias_zone_type_attr_handler,
[IASZone.attributes.ZoneStatus.ID] = ias_zone_status_attr_handler
},
[PowerConfiguration.ID] = {
[PowerConfiguration.attributes.BatteryVoltage.ID] = battery_level_handler
}
},
cluster = {
[IASZone.ID] = {
[IASZone.client.commands.ZoneStatusChangeNotification.ID] = ias_zone_status_change_handler
}
}
},
lifecycle_handlers = {
added = ias_device_added,
doConfigure = configure_handler,
infoChanged = ias_info_changed
},
ias_zone_configuration_method = constants.IAS_ZONE_CONFIGURE_TYPE.AUTO_ENROLL_RESPONSE
}

defaults.register_for_default_handlers(zigbee_generic_sensor_template, zigbee_generic_sensor_template.supported_capabilities)
local zigbee_sensor = ZigbeeDriver("zigbee-sensor", zigbee_generic_sensor_template)
zigbee_sensor:run()
Loading
Loading