-
Notifications
You must be signed in to change notification settings - Fork 522
WWSTCERT-9096 Ct clamp driver wwsm #2552
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
base: main
Are you sure you want to change the base?
WWSTCERT-9096 Ct clamp driver wwsm #2552
Conversation
|
|
||
| local function battery_level_handler(driver, device, value, _zb_rx) | ||
| if type(value.value) == "number" then | ||
| local number = value.value/2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spacing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now corrected
PR SmartThingsCommunity#2552 indent code under the statement to the right as per pull request comment
pegor-karoglanian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Invitation URL: |
| - id: "ChameleonCTClamp" | ||
| deviceLabel: Zigbee CT | ||
| deviceIdentifiers: | ||
| - 0x000D | ||
| zigbeeProfiles: | ||
| - 0x0104 | ||
| clusters: | ||
| server: | ||
| - 0x0000 #Basic | ||
| - 0x0001 #Power Configuration | ||
| - 0x0002 #Device Temperature Configuration | ||
| - 0x0003 #Identify | ||
| - 0x0019 #Over the Air Bootloading | ||
| - 0x0020 #Poll Control | ||
| - 0x0702 #Simple Metering | ||
| - 0x0B04 #Electrical Measurement | ||
| client: | ||
| - 0x0003 #Identify | ||
| deviceProfileName: ct-clamp | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please just include the manufacturer-specific fingerprint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other fingerprints now removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes to this file should be omitted
Test Results 71 files 469 suites 0s ⏱️ Results for commit e04499e. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against e04499e |
drivers/SmartThings/zigbee-power-meter/profiles/power-energy-battery-temperature.yml
Show resolved
Hide resolved
| local batt_level = device:get_latest_state("main", capabilities.battery.ID, capabilities.battery.battery.NAME) or nil | ||
| if batt_level == nil then | ||
| device:emit_event(capabilities.battery.battery.normal()) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect this to go in added. Is there a reason you included it in init?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now removed as unnecessary
| local function battery_level_handler(driver, device, value, _zb_rx) | ||
| if type(value.value) == "number" then | ||
| local number = value.value/2 | ||
| local integer_result = math.floor(number) | ||
| device:emit_event(capabilities.battery.battery(integer_result)) | ||
| else | ||
| log.error("Invalid battery level value received: " .. tostring(value.value)) | ||
| end | ||
| end | ||
|
|
||
| local function temperature_handler(driver, device, value, _zb_rx) | ||
| if type(value.value) == "number" then | ||
| device:emit_event(capabilities.temperatureMeasurement.temperature({ value = value.value, unit = "C" })) | ||
| else | ||
| log.error("Invalid temperature value received: " .. tostring(value.value)) | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you just added battery and temperatureMeasurement here: https://github.com/SmartThingsCommunity/SmartThingsEdgeDrivers/blob/main/drivers/SmartThings/zigbee-power-meter/src/init.lua#L53
You could omit most of this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've moved battery but temperatureMeasurement wasn't possible to move as we think it's a different ZCL temperature attribute from a different cluster to what you were thinking of @greens but we may have missed something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct. I see now that you are using clusters.DeviceTemperatureConfiguration rather than clusters.TemperatureMeasurement, so you are correct, that is not handled by our defaults.
|
greens
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address the remaining comments.
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made suggested change and removed newlines
| {model = "CT101xxxx" } | ||
| } | ||
|
|
||
| -- temperature: 0.5C, battery level remaining: 1% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment still needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now removed
| for _, fingerprint in ipairs(ZIGBEE_FINGERPRINT) do | ||
| if device:get_model() == fingerprint.model then | ||
| return true | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: spacing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adjusted spacing
| { | ||
| cluster = PowerConfiguration.ID, | ||
| attribute = PowerConfiguration.attributes.BatteryPercentageRemaining.ID, | ||
| minimum_interval = 30, | ||
| maximum_interval = 3600, | ||
| data_type = PowerConfiguration.attributes.BatteryPercentageRemaining.base_type, | ||
| reportable_change = 2 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend that you omit this and use the defaults here, which has a maximum_interval of 6 hours and a reportable change of 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
omitted and using defaults
| local function battery_level_handler(driver, device, value, _zb_rx) | ||
| if type(value.value) == "number" then | ||
| local number = value.value/2 | ||
| local integer_result = math.floor(number) | ||
| device:emit_event(capabilities.battery.battery(integer_result)) | ||
| else | ||
| log.error("Invalid battery level value received: " .. tostring(value.value)) | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since you are using the defaults here now, this function can be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function removed
| if type(value.value) == "number" then | ||
| device:emit_event(capabilities.temperatureMeasurement.temperature({ value = value.value, unit = "C" })) | ||
| else | ||
| log.error("Invalid temperature value received: " .. tostring(value.value)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this log statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log statement removed
|
Last 4 comments have been actioned. Test summary for Chameleon CT Clamp Congratulations! Your product passed the test and no issues were detected with your product. |
|
You'll need to rebase your branch to resolve the merge conflicts. |
|
Regarding the conflicts the fingerprints.yml is just an additional of chameleon along side the BITUO TECHNIK entry. but my init.lua originally had sub_drivers = { but currently the current master/main has sub_drivers = require("sub_drivers"), with a new file sub_drivers.lua and also a new file lazy_load_subdriver.lua this looks like a newer feature for sub drivers, I am right in assuming I will need to implement this before we can merge? |
…driver for temperature, battery percentage e
battery seems to always be at 100% on both tested clamps, even throiugh one is as 98%
Now calling the ct clamp sub driver - have temperature,battery,power meter and energy meter on the screen, last two are correct values, first two are not as yet
…nd deployed to the hub, using the standard smartthings zigbee power meter as the base and adding as a sub driver the temperature and battery level
PR SmartThingsCommunity#2552 indent code under the statement to the right as per pull request comment
rename file from ct-clamp.yml to power-energy-battery-temperature.yml add copyright information
removed unused variables cluster_base and data_types
add changed name into yaml file remove non manufacturer specific items
Added longer copyright at top of our init.lua file removed comment about tempeature and battery removed battery level handler to main init.lua file under capabilities.battery, temperature syayed as its cluster 0x0402 not the normal temperature cluster and didn't update the temperature
removed 3 lines as requested in comments on the pull request
remove unintentional extra line in .gitignore
add new line, that was erased by accicent in these two files
trying to insert a new line at the end of the file to remove the red circle in github, when the file is checked in
remove battery percentage config, and use defaults remove battery_level_handler as usinf the capabilites function remove log function in temperature handler adjust spacing in is_chameleon_ct_clamp function
after a rebase add in the chameleon entry into the sub_drivers.lua file
171237c to
0638d49
Compare
after a rebase add in the chameleon entry into the sub_drivers.lua file add in the new files can_handle.lua and fingerprints.lua, for the new type of sub driver architecture
Check all that apply
Type of Change
Checklist
Description of Change
add a sub driver to the standard edge driver for Zigbee Power meter to supply temperature and battery percentage readings
Summary of Completed Tests
Smartthings full 14 test test suite from the certification console, all 14 tests passed
Test date: 13 November 2025 at 11:28:28
Test duration: 00:12:27
Session ID: WHJ1WLTHID
Test date: 13 November 2025 at 11:51:55
Test duration: 00:09:10
Session ID: 1DIHUP08CV