[BUG] [controller] Missing support for TemperatureSetpointHold on client-side.

[turon]

Reproduction steps

See appclusters spec 4.3.7. Attributes (HVAC.Thermostat.TemperatureSetpointHold = 0x24).
Notice device-side implementation.
Notice missing client-side implementation (no support for "code": 36).

Bug prevalence

always

GitHub hash of the SDK that was being used

master

Platform

core

Platform Version(s)

No response

Anything else?

No response

[bzbarsky-apple]

@turon This is supported fine in chip-tool, Darwin framework, darwin-framework-tool.

It's possible that the java codegen and maybe the python codegen actually pay attention to the enabled state of things in controller-clusters.zap. We should fix them so they stop paying attention, like all the other clients.

[turon]

@bzbarsky-apple Wouldn't the proper fix be to set the enable correctly in the XML? Or from your comment, should I take it that there are so many fields with enabled set incorrectly that it is effectively a dead field?

I don't see TemperatureSetpointHold in the client XML either.

[bzbarsky-apple]

Wouldn't the proper fix be to set the enable correctly in the XML?

"enable" is not set in XML. It's set in the .zap file.

Past that, define "correctly"? If the intent for the client is "expose everything", examining "enabled" just adds a point of failure, and we had repeated bugs with people forgetting to enable things in controller-clusters.zap when adding something new to the XML.

So yes, it's basically a dead field in practice, as far as chip-tool and the Darwin framework and darwin-framework-tool are concerned.

If Java and Python want to "expose everything" they should stop examining it too. If they want to have an allow-list of things they expose, then their maintainers need to watch for all changes to XML and update the .zap file as needed.

I don't see TemperatureSetpointHold in the client XML either.

That's not XML. That's a .zap file. And yes, it's not listed there, probably because no one touched its state on that cluster in that .zap file after the attribute was added to the XML.

[andy31415]

@bzbarsky-apple do we have a list of when we should or we should not be paying attention to fields?

it sounds to me like client-side cluster generation should not be paying attention (because a client should be able to chose to query anything?) and a server-side should (to generate the right callbacks)?

What are the macros that care (or do not care) about enabling in zap?

[andy31415]

I believe this is fixed now.