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

Feat: Add CDT Support as CA (Configuration Appliance) Client Actor #132

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

daviddsapir
Copy link

@daviddsapir daviddsapir commented Oct 30, 2024

Overview

This commit introduces CDT support as a CEM client actor.

It provides full support for the CDT use case, specifically Scenario 1. This includes the ability to retrieve setpoints, their constraints, and map them to their corresponding operation modes via HvacSetpointRelations.

For the use case to fully function, support for the CDSF (Configuration of DHW System Function) use case is necessary.
Specifically, we need to request HvacOperationModeDescriptionListDataType, which is used in conjunction with HvacSystemFunctionSetpointRelationListDataType to establish the mapping between operation modes and their setpoints, and to enable the ability to write setpoints.

❗Writing setpoints was tested and confirmed to work with Vaillant's HeatPump by requesting the HvacOperationModeDescriptionListDataType message and performing the mapping without the CDSF use case.

Mapping Setpoints to Operation Modes

This commit introduces a mapping between Operation Modes and setpoints, enabling retrieval of the setpoint based on a specified Operation Mode. According to the SPEC, the following rules apply for each mode:

  • Auto: If supported in this scenario, this mode must relate to one to four temperature setpoints.
  • Off: If supported in this scenario, this mode must relate to either no setpoints or exactly one temperature setpoint.
  • On: This mode must relate to exactly one temperature setpoint.
  • Eco: This mode must relate to exactly one temperature setpoint.

Relations between system functions, operation modes and setpoints.

  • HVAC operation modes are used to describe which behaviour the HVAC system functions have.
    Each HVAC operation mode may be used by several HVAC system functions.
  • Each supported operation mode can relate to one ore more temperature setpoints.

Diagram example:

image

Therefore, in order to get the operation modes and their setpoints we need to:

  1. Obtain the SystemFunctionId for the “DHW” system function.
  2. Retrieve the operation modes associated with this system function ID.
  3. Map the setpoints to each operation mode to get a map in which we can get the setpoint by mode.

Required Messages for Mapping:

  • HvacSystemFunctionDescriptionListData: Retrieves the SystemFunctionId for the “DHW” system function.
  • HvacSystemFunctionSetpointRelationData: Provides the relationships between operation modes and setpoints for the “DHW” system function.
  • HvacOperationModeDescriptionListData: Supplies the operation mode descriptions for the operation modes in the relations.

⚠️ Only the HvacSystemFunctionSetpointRelationData is request in the CDT usecase. The other 2 required messages are requested in the CDSF usecase scenario 1. This is why this usecase depends on other use cases: CDSF (Configuration of DHW System Function) and MDT (Monitoring of DHW Temperature).

Resources used (specifications):

  • EEBus UC Technical Specification Configuration of DHW Temperature
  • EEBus SPINE Technical Specification Resource Specification

@daviddsapir daviddsapir force-pushed the davidsa/feat/add-cdt-usecase branch 3 times, most recently from a528830 to 6a7a862 Compare October 30, 2024 08:07
timePeriod = *setpoint.TimePeriod
}

isActive := (setpoint.IsSetpointActive == nil || *setpoint.IsSetpointActive)
Copy link
Author

@daviddsapir daviddsapir Oct 30, 2024

Choose a reason for hiding this comment

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

According to the spec., the following rules apply:

  • isSetpointChangeable - If set to "true" the server SHALL accept changes of the setpoint by appropriate clients (e.g. the bound client). If set to "false" the server SHALL decline change requests. If absent, the server SHOULD accept changes.
  • isSetpointActive - If set to "false", the setpoint is inactive. If set to "true" or omitted, the setpoint is active.

Ref: [Resource Specification] 4.3.23.4 setpointListData.

Copy link
Member

Choose a reason for hiding this comment

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

Is this a question or a note to yourself? :)

Copy link
Author

Choose a reason for hiding this comment

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

This is a note for you TBH 🙂

To clarify why I used the || operator:

In some parts of EEBus, for boolean fields, an absent (nil) value is considered false, so you typically check it like this: isActive := (setpoint.IsSetpointActive != nil && *setpoint.IsSetpointActive)

However, in this case, the specification requires the opposite: if the value is absent, it should default to true.

Copy link
Member

Choose a reason for hiding this comment

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

Hehe 😃

Yeah, consistency usually wins, and hurts when it is not present. Would you mind adding this note as a comment into the code? It is nearly impossible to keep this in mind and always check the specs or find this note.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

Added this note as a comment into the code:

// As per [Resource Specification] 4.3.23.4 setpointListData:
// - isSetpointActive: If false, the setpoint is inactive; if true or omitted, it is active.
// - isSetpointChangeable: If true, the server accepts changes; if false, it declines changes. If absent, changes are accepted.
isActive := (setpoint.IsSetpointActive == nil || *setpoint.IsSetpointActive)
isChangeable := (setpoint.IsSetpointChangeable == nil || *setpoint.IsSetpointChangeable)

@daviddsapir daviddsapir force-pushed the davidsa/feat/add-cdt-usecase branch from 6a7a862 to c9558d4 Compare October 30, 2024 08:52
@daviddsapir daviddsapir changed the title Feat: Add CDT Support as CEM Client Actor Feat: Add CDT Support as CA Client Actor Oct 30, 2024
@daviddsapir daviddsapir force-pushed the davidsa/feat/add-cdt-usecase branch from c9558d4 to a1b02a4 Compare October 30, 2024 08:53
@daviddsapir daviddsapir changed the title Feat: Add CDT Support as CA Client Actor Feat: Add CDT Support as CA (Configuration Appliance) Client Actor Oct 30, 2024
@daviddsapir daviddsapir force-pushed the davidsa/feat/add-cdt-usecase branch 3 times, most recently from 47ebb27 to f9e823c Compare October 30, 2024 09:05
@daviddsapir daviddsapir force-pushed the davidsa/feat/add-cdt-usecase branch 10 times, most recently from e4322d5 to e604a20 Compare October 31, 2024 21:23
@daviddsapir daviddsapir marked this pull request as draft November 1, 2024 07:20
@daviddsapir daviddsapir force-pushed the davidsa/feat/add-cdt-usecase branch 6 times, most recently from f7ad0a6 to c1ba25b Compare November 3, 2024 09:01
@daviddsapir daviddsapir marked this pull request as ready for review November 3, 2024 12:12
@daviddsapir daviddsapir force-pushed the davidsa/feat/add-cdt-usecase branch from c1ba25b to 0b7f066 Compare November 3, 2024 15:10
@DerAndereAndi
Copy link
Member

DerAndereAndi commented Nov 4, 2024

Thanks a lot for this addition! Looks good on first glance. Before I go into details (which will take some time), would it be possible to add more tests on the added functionality. Both in the new client and the use case implementation? I wouldn't want the coverage to drop ;)

In addition, is it possible to get some SPINE trace logs to see the actual code in action? This would help me a lot checking the implementation.

This commit introduces CDT support as a CA (Configuration Appliance)
client actor.

It provides full support for the CDT use case, specifically Scenario 1.
This includes the ability to retrieve setpoints, their constraints, and
map them to their corresponding operation modes via
HvacSetpointRelations.

For the use case to fully function, support for the CDSF (Configuration
of DHW System Function) use case is necessary. Specifically, we need to
request HvacOperationModeDescriptionListDataType, which is used in
conjunction with HvacSystemFunctionSetpointRelationListDataType to
establish the mapping between operation modes and their setpoints, and
to enable the ability to write setpoints.

Note: Writing setpoints was tested and confirmed to work with Vaillant's
HeatPump by requesting the
HvacOperationModeDescriptionListDataType message and performing the
mapping without the CDSF use case.

Resources used (specifications):
- EEBus UC Technical Specification Configuration of DHW Temperature
- EEBus SPINE Technical Specification Resource Specification
@daviddsapir daviddsapir force-pushed the davidsa/feat/add-cdt-usecase branch from 0b7f066 to 26efd39 Compare November 4, 2024 12:00
@daviddsapir
Copy link
Author

no problem, I'll do both :)

@daviddsapir daviddsapir marked this pull request as draft November 5, 2024 14:49
@daviddsapir daviddsapir force-pushed the davidsa/feat/add-cdt-usecase branch 2 times, most recently from dc118c1 to 05f7b21 Compare November 11, 2024 13:56
@daviddsapir daviddsapir force-pushed the davidsa/feat/add-cdt-usecase branch from 05f7b21 to bb7e19e Compare November 14, 2024 11:21
@daviddsapir daviddsapir force-pushed the davidsa/feat/add-cdt-usecase branch from bb7e19e to 4afe071 Compare November 14, 2024 11:23
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