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

Update candeo.ts #8147

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update candeo.ts #8147

wants to merge 2 commits into from

Conversation

dhc25
Copy link
Contributor

@dhc25 dhc25 commented Oct 16, 2024

Adding Candeo C-ZB-SM205-2G

Adding Candeo C-ZB-SM205-2G
@dhc25
Copy link
Contributor Author

dhc25 commented Oct 16, 2024

@Koenkk Can you please help with this lint formatting error again please? Still can't see what we're getting wrong here...
Thanks

await reporting.bind(endpoint2, coordinatorEndpoint, ['genOnOff']);
await reporting.onOff(endpoint1);
await reporting.onOff(endpoint2);
await endpoint1.read('genOnOff', [0x0000]);
Copy link
Owner

Choose a reason for hiding this comment

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

Why are these reads needed?

@dhc25
Copy link
Contributor Author

dhc25 commented Oct 17, 2024

Hi @Koenkk - to give the user a better out-of-the-box experience, and ensuring that the states of the device are accurately presented straight away.

@Koenkk
Copy link
Owner

Koenkk commented Oct 17, 2024

But how are these values exposes? Because I see no corresponding expose and fromZigbee converter.

@dhc25
Copy link
Contributor Author

dhc25 commented Oct 18, 2024

Sorry, we're not really sure what specifically you're asking.
You're referring to lines 187-192, correct? These are for OnOff cluster attributes for fz.on_off and fz.power_on_behavior.
Maybe we're not understanding the question...?

Can you please give us more info if there's a better way to do this, or if we've added something that looks incorrect to you?

@Koenkk
Copy link
Owner

Koenkk commented Oct 18, 2024

I cleaned the code a bit, can you check if everything is still OK?

@dhc25
Copy link
Contributor Author

dhc25 commented Oct 18, 2024

Thanks for taking a look at this.

A problem we have is that a converter using modern extend does not offer powerOnBehaviour correctly for multi endpoint devices.

Initial view of your changes - it looks like -

  1. Power on behaviour isn't configurable on each endpoint with modernExtend
  2. State aren't pre-populated
  3. The electrical measurement and metering clusters are on endpoint 11
  4. Reporting cannot be customised to report more frequently when there is no activity.

@Koenkk
Copy link
Owner

Koenkk commented Oct 19, 2024

  1. This is supported now:
    exposes.push(...exposeEndpoints(e.power_on_behavior(['off', 'on', 'toggle', 'previous']), args.endpointNames));
  2. It should do that here: , could you provide the debug log when configuring the device?
  3. What problems do you experience with this?
  4. It is possible to change it, but we want to standardise the reporting interval for all devices as much as possible. If for any specific use case you want to have more specific reporting, you can change that via the z2m frontend.

@dhc25
Copy link
Contributor Author

dhc25 commented Oct 21, 2024

  1. Ok great - we can't replicate your suggestion in what we're currently using. (Home Assistant add-on, 1.40.2, zigbee-herdsman-converters version 20.21.0)

  2. Again, as above we can't replicate that. Is this also something that has been fixed in the latest updates, and yet to be rolled out?

  3. The electrical measurement is on endpoint 11, which has been removed in your edits

  4. We would like to pre-set the reporting values for our devices, as it seems that 65000 seconds is too long. Although we know customers can change this themselves, our approach is to set the device up in pairing as best we can, so the customer does not have to edit the reporting settings.

Would it be possible to go with the code we have written? We have tested this and we are happy that it works with our device, providing the features that the device supports.

If the updates to modernExtend in the next rollout fix issues 1 and 2 on this list, we will edit the code to use modernExtend instead.

Thanks

@Koenkk
Copy link
Owner

Koenkk commented Oct 21, 2024

  1. You need to use the dev branch or wait for the 1 November release
  2. Should be OK in 1.40.2, could you provide the debug log when configuring the device?
  3. Does anything break because of this?
  4. 65000 is the max reporting interval, if the value changes it should immediately report the change, is this not the case?

@dhc25
Copy link
Contributor Author

dhc25 commented Oct 21, 2024

Thanks.

We don't have a dev branch setup, so can't test your responses to 1, 2, 3 of our observations.

  1. Yes - reporting is immediate if the value changes, but we want a shorter reporting time if the value does not change. Why is not possible?

As per my previous message, is there a good reason that we cannot go ahead with our tested and working code?

And then we can modify the code once we've tested on the 1 November release.

@Koenkk
Copy link
Owner

Koenkk commented Oct 22, 2024

We don't have a dev branch setup, so can't test your responses to 1, 2, 3 of our observations.

Switching to the dev branch is documented here: https://www.zigbee2mqtt.io/advanced/more/switch-to-dev-branch.html, does that help?

Yes - reporting is immediate if the value changes, but we want a shorter reporting time if the value does not change. Why is not possible?

If the value doesn't change, why do you want to have it reported again?
By using standard values we provide a consistent experience for all devices, that's why I'm pushing for the default values.

@dhc25
Copy link
Contributor Author

dhc25 commented Oct 22, 2024

Thanks.

I understand, but 18 hours is a long time, and we would prefer a more regular reporting.

I am not able to switch to dev branch on my current set up.

I go back to my earlier question - is there a good reason that we cannot go ahead with our tested and working code? Is it simply because you want more slimline code?

We will slim it down after the November release if everything works as you're saying... Is that ok?

@Koenkk
Copy link
Owner

Koenkk commented Oct 22, 2024

We will slim it down after the November release if everything works as you're saying... Is that ok?

Ok then let's do that! Could you update the PR again?

@dhc25
Copy link
Contributor Author

dhc25 commented Oct 22, 2024

Thank you for this. We will be sure to look at again after the next roll out.

Sorry - I don't think I've done this correctly. Please forgive the newbie approach to Github.
I've opened another PR that needs conflicts resolving

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants