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

Chilled Beams #725

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

Chilled Beams #725

wants to merge 11 commits into from

Conversation

nwssullivan
Copy link

New extension for chilled beams

Adding chilled beam telemetry fields
Adding descriptor for chilled beams field
@google-cla
Copy link

google-cla bot commented Jul 25, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@cstirdivant cstirdivant self-assigned this Jul 25, 2022
@cstirdivant cstirdivant requested a review from tasodorff August 19, 2022 20:52
@cstirdivant cstirdivant removed their assignment Aug 19, 2022
uses:
- chilled_water_valve_percentage_command
- zone_air_temperature_sensor
- zone_air_occupied_temperature_setpoint
Copy link
Collaborator

Choose a reason for hiding this comment

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

Kind of odd seeing occupied temp setpoint and unoccupied cooling setpoint. shouldnt they both be cooling setpoints?

uses:
- perimeter_heating_water_valve_percentage_command
- zone_air_temperature_sensor
- zone_air_occupied_temperature_setpoint
Copy link
Collaborator

Choose a reason for hiding this comment

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

same comment as above, except heating. id expect these to match.

- zone_occupancy_status
uses:
- zone_air_temperature_sensor
- zone_air_occupied_temperature_setpoint
Copy link
Collaborator

Choose a reason for hiding this comment

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

its odd to me that you have differentiated heating and cooling setpoints for unoccupied times, but there are no equivalent differentiations for occupied setpoints.

in general, we see either a single setpoint that changes based on mode, or we see two effective setpoints (for cooling and for heating) that change based on occ/unocc status. what is the context for this weird configuration?

Copy link
Author

Choose a reason for hiding this comment

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

There were no heating/cooling setpoints for occupied mode. Unit runs in either cooling or heating mode while zone is occupied and controls to the occupied setpoint. In unoccupied mode the heating/cooling valves modulate to maintain the zone between the unoccupied heating/cooling setpoints.

Copy link
Collaborator

Choose a reason for hiding this comment

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

can this be fixed? this inconsistency between control in occupied and unoccupied mode is highly irregular.

otherwise, these will need to be modeled separately (e.g. zone temp and occ setpoint together in one abstract type, and zone temp and unoccupied setpoints together in another -- then a single type can implement both pieces of functionality in some non-canonical way, since this is very irregular).

Copy link
Author

Choose a reason for hiding this comment

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

I see this pretty frequently in terminal units where there is a single setpoint during occupied hours, but during unoccupied hours there is a night setback for both heating and cooling. Do they need to be separated into separate abstract types?

@mschulze17 mschulze17 assigned nwssullivan and unassigned tasodorff Aug 30, 2022
- zone_occupancy_status
uses:
- zone_air_temperature_sensor
- zone_air_occupied_temperature_setpoint
Copy link
Collaborator

Choose a reason for hiding this comment

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

can this be fixed? this inconsistency between control in occupied and unoccupied mode is highly irregular.

otherwise, these will need to be modeled separately (e.g. zone temp and occ setpoint together in one abstract type, and zone temp and unoccupied setpoints together in another -- then a single type can implement both pieces of functionality in some non-canonical way, since this is very irregular).

## New fields for chilled beams
- zone_air_occupied_temperature_setpoint
- zone_air_unoccupied_cooling_temperature_setpoint
- perimeter_heating_water_valve_percentage_sensor
Copy link
Collaborator

Choose a reason for hiding this comment

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

will this ever exist independently of another valve?

if we know the device lives on a perimeter then would heating_water_valve_percentage_... be sufficient?

Copy link
Author

Choose a reason for hiding this comment

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

Potentially there could be other heating valves, like reheat or four pipe chilled beams.

@mschulze17 mschulze17 added the P2 Medium priority (default) label Apr 17, 2023
@cstirdivant cstirdivant added the Ontology Model Extensions or edits to the ontology model label Jan 12, 2024
@shambergoldstein
Copy link
Collaborator

@nwssullivan are the changes in this PR still needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ontology Model Extensions or edits to the ontology model P2 Medium priority (default)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants