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

@H3buss Added measured temperature SET for CCTFR6700 #2558

Merged
merged 40 commits into from
May 22, 2021
Merged

@H3buss Added measured temperature SET for CCTFR6700 #2558

merged 40 commits into from
May 22, 2021

Conversation

H3buss
Copy link
Contributor

@H3buss H3buss commented May 8, 2021

Requires report capability from herdsman

if (systemMode === undefined) {
systemMode = utils.getKey(legacy.thermostatSystemModes, value, value, Number);
}
globalStore.putValue(entity, 'systemMode', systemMode);
Copy link
Owner

Choose a reason for hiding this comment

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

Is the only reason for this to buffer the commands until the device awakes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These attributes are read by the device, they cannot be written.

Copy link
Owner

Choose a reason for hiding this comment

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

What if we just send a readReponse here? (without the device requested for one). Does that also work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the device is asleep, it doesn't acknowledge any frame.

Also, spamming readResponses to the device does not seem to work either, even when awake. The device seems to correlate the responses to its requests somehow (with sequence number maybe?)

Copy link
Owner

Choose a reason for hiding this comment

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

I've updated herdsman to allow read response to all attributes. Can you check if it works? (update to the latest dev branch first). After that:

  • Remove the read response code from the onEvent handler
  • Replace globalStore.putValue(entity, 'systemMode', systemMode); with endpoint.saveClusterAttributeKeyValue('hvacThermostat', {systemMode: systemMode})

This has the benefit that these attributes are persisted when z2m is restarted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Do I still need the hacked firmware/modified startZnp of herdsman for this to work?

  2. I'm not sure how to update herdsman to the latest dev version within my zigbee2mqtt npm environment.

Copy link
Owner

Choose a reason for hiding this comment

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

  1. Firmware yes, modified startZnp no
  2. Just follow https://www.zigbee2mqtt.io/how_tos/how-to-switch-to-dev-branch.html#bare-metal again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested and it works, I've updated the code accordingly.
I still need to use globalStore for pending writes to keypad_lockout and for handling of schneider_ui_action where I need to know if screen is awake or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry to come back on that issue, but I installed my production environnement and the the thermostat no longer take the readResponses into account. I think I messed-up when I tested with the hacked firmware a while ago.

After some troubleshooting, the readResponses are sent from coordinator endpoint 3, and the sent attributes are correct, however the profile is incorrect: should be 0x0104 for Home Automation, and is 0x0105.

How can I set the profile for the read responses?

Koenkk added a commit to Koenkk/zigbee-herdsman that referenced this pull request May 17, 2021
type: ['attributeReport', 'readResponse'],
convert: (model, msg, publish, options, meta) => {
const temperature = parseFloat(msg.data['measuredValue']) / 100.0;
const property = postfixWithEndpointName('temperature', msg, model);
Copy link
Owner

Choose a reason for hiding this comment

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

Why do we have a temperature when there is already a local_temperature? (we also do not do this for other thermostats)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The temperature appear as a sensor on home assistant, which helps automation. But I concur that this is not really needed: the temperature could be reported on local temperature only.

Copy link
Owner

Choose a reason for hiding this comment

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

The temperature can also be retrieved from the thermostat, it's better not to have duplicated data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the temperature key and expose.

converters/toZigbee.js Outdated Show resolved Hide resolved
converters/toZigbee.js Outdated Show resolved Hide resolved
Koenkk added a commit to Koenkk/zigbee-herdsman that referenced this pull request May 19, 2021
converters/fromZigbee.js Outdated Show resolved Hide resolved
converters/toZigbee.js Outdated Show resolved Hide resolved
lib/store.js Outdated Show resolved Hide resolved
@Koenkk
Copy link
Owner

Koenkk commented May 21, 2021

Did some small updates, can you check if everything is OK? Action should be in snake case now instead of camel case. If ok this can be merged.

@H3buss
Copy link
Contributor Author

H3buss commented May 22, 2021

Tested, and it works.

Will you also publish the modified firmware?

@Koenkk Koenkk merged commit d0fb06c into Koenkk:master May 22, 2021
@Mega-Touk
Copy link

Mega-Touk commented Jan 17, 2022

Hello @H3buss
Firstly thanks for your work (& Koenkk too)

Do you have any idea why energy is never reported by the CCTFR6700 ?
I always get payload that look like this: '{"energy":null,"......'
I also have some "No converter available" messages, maybe this is related ?

debug No converter available for 'CCTFR6400' with cluster 'haDiagnostic' and type 'attributeReport' and data '{"lastMessageLqi":216,"lastMessageRssi":-46}'
debug No converter available for 'CCTFR6400' with cluster 'haDiagnostic' and type 'attributeReport' and data '{"57345":724}'

Also the documentation is mentioning a bind from a sensor, here I don't really get how to implement this (i'm also using a CCTFR6400). I'm using an automation for now but it feel a bit heavy. Could we make the documentation easier to understand and apply ?

Sorry if I'm not using the right place to discuss about this but I'm not sure if i should open a new issue or not.

@H3buss
Copy link
Contributor Author

H3buss commented Jan 17, 2022

I don't know why the device do not report the energy. I'm pretty sure I got it working at some point during my tests but now it doesn't for some reason. I tried to bind the attribute manually again, to no avail. The debug message you see are from haDiagnostic cluster and I don't see how they could be related to this issue.
On HA I use integration of power to compute energy as a fallback. This is not pretty but works.

As all my devices are now used in my production environment, I cannot work on this without braking part on my home automation. Not great for the WAF ;).

As for the reporting of ambiant temperature from a sensor, I agree that the documentation is unclear. The important missing part is the topic you need to publish the temperature to: temperature_measured_value.
In the documentation, I should remove the "bind" part. I never tested it and doubt it would work. Using an automation is the way to get it working.
On HA, I use a custom component to create a virtual Climate entity that links one CCTFR6400 to one or more CCTFR6700. It performs the measure temperature synchonisation from the CCTFR6400 to the CCTFR6700, but also sends the pi_heating_demand back to display the heating state.
My code is realy not pretty but I can share it if necessary.

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.

None yet

3 participants