-
Notifications
You must be signed in to change notification settings - Fork 682
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
Add Lixee ZLinky_TIC manufacturer specific cluster #1165
Add Lixee ZLinky_TIC manufacturer specific cluster #1165
Conversation
Codecov ReportBase: 80.00% // Head: 80.02% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## dev #1165 +/- ##
==========================================
+ Coverage 80.00% 80.02% +0.02%
==========================================
Files 239 239
Lines 7430 7438 +8
==========================================
+ Hits 5944 5952 +8
Misses 1486 1486
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Pull Request Test Coverage Report for Build 3219067147
💛 - Coveralls |
db467d0
to
2367488
Compare
zhaquirks/lixee/zlinky.py
Outdated
name = "ZLinky_TIC Manufacturer specific" | ||
ep_attribute = "zlinky_cluster" | ||
manufacturer_attributes = { | ||
0x0000: ("optarif_or_ngtf", t.LimitedCharString(16)), |
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.
Should the attribute name be translated from French to English? Doing so would be harder to maintain.
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.
Hrm, @Adminiuga what do you think? My initial thought is yes and we could have a comment with the French translation on each line to help with the maintenance… the entire code base is in English so having just this in French would be a bit odd IMO
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.
Sounds good to me, I'll proceed with the translation.
2367488
to
a57ebf7
Compare
23df8ef
to
67956d5
Compare
67956d5
to
b8c9bed
Compare
any news about that Merge Request ? Still in DRAFT ? |
Hi, it is not working yet, querying the manufacturer specific clusters is failing. I believe there are firmware related issues to be resolved before, Edit: my ZLinky_TIC issues were resolved by manufacturer, it was an hardware issue. |
b8c9bed
to
5d650b1
Compare
Hi, maybe reading error is related to this : Koenkk/zigbee2mqtt#11431 As i understand, reading is not working on this cluster when manufacturer code is specified. |
Indeed, that is the firmware related issue I'm referring to. |
Any news on this? |
FYI : OTA 6.0 released |
5d650b1
to
6c4f1af
Compare
How can I help on this ? |
The issue when reading manufacturer specific cluster attributes with the manufacturer id specified seems to be still present with firmware v6.0. Invoking the following with ZHA Toolkit with the manufacturer id:
Does not work:
Without the manufacturer id:
It works:
Anyway, I'm downgrading the firmware to v5.0 because v6.0 is causing issues. |
Apparently v7.0 has been released, does it help with this in any way? |
Sadly, not. |
This pre-release fix the manufacturer issue : https://github.com/fairecasoimeme/Zlinky_TIC/releases/tag/v9.0 |
Awesome, thanks! OTA in progress... |
I can confirm it is now working fine:
I'll update this PR ASAP. |
6c4f1af
to
a15a5ef
Compare
ac43627
to
0458f1d
Compare
0458f1d
to
50f7e66
Compare
0x0000: ( | ||
"hist_tariff_option_or_std_supplier_price_schedule_name", | ||
t.LimitedCharString(16), | ||
True, | ||
), |
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.
It would be awesome to have descriptions on attributes that would show up in the UI, e.g.:
0x0000: ( | |
"hist_tariff_option_or_std_supplier_price_schedule_name", | |
t.LimitedCharString(16), | |
True, | |
), | |
0x0000: ( | |
"hist_tariff_option_or_std_supplier_price_schedule_name", | |
t.LimitedCharString(16), | |
True, | |
"Historical mode: Option tarifaire / Standard mode: Nom du calendrier tarifaire fournisseur", | |
), |
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.
@puddly whatcha think about this request?
I believe this PR is now ready for another round of reviews. 🙏 |
e799d48
to
ed6b208
Compare
Nice! By the way, would be great if someone wrote ZLinky TIC OTA Provider download code for zigpy, see -> zigpy/zigpy#1060 |
… standard modes respectively
ed6b208
to
d718950
Compare
Thanks @dmulcahey! 🎉 |
I have Home Assistant (version: 2022.10.5) with Zlinky (firmware 9 out-of-the-box) and I checked the ZLinkyTICManufacturerCluster. All is ok |
This PR adds support for the manufacturer specific cluster
0xFF66
of the Lixee ZLinky_TIC device.Reference available at:
Submitted as draft as there's only one of many attribute for now.
Resolves #1146