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

New devices HDS, WSR, etc #794

Merged
merged 118 commits into from
Oct 4, 2023
Merged

Conversation

josephedwardchang
Copy link
Contributor

new devices:
HDS - hydrogen sensor with alarm
WSR - water softener
WCR - water conditioner
EHT - heat tracer

EHT_SS and EHT_SS_ALM - new canonicals for EHT
FCU_RHDHS_DTM_DFSS_STM_DTC_SHM_SFVSC_ALM_SS_SRC - new canonical FCU
UPS_SS_ALM - new canonical UPS

added ALM as tag for alarms in SAFETY/Abstract.yaml, ELECTRICAL/ and HVAC/
added primary_fire_alarm: # better than using fire_alarm_1 (which will lead to fire_alarm_1_23)
           ..also added secondary_fire_alarm
added apparentpower_sensor (and reactivepower_sensor) because apparent_power_sensor has error on units 
          and can't be resolved but using these new ones has no error.

ontology/yaml/resources/fields/telemetry_fields.yaml Outdated Show resolved Hide resolved
ontology/yaml/resources/fields/telemetry_fields.yaml Outdated Show resolved Hide resolved
ontology/yaml/resources/fields/telemetry_fields.yaml Outdated Show resolved Hide resolved
ontology/yaml/resources/HVAC/entity_types/ABSTRACT.yaml Outdated Show resolved Hide resolved
ontology/yaml/resources/SAFETY/entity_types/WTR.yaml Outdated Show resolved Hide resolved
ontology/yaml/resources/fields/telemetry_fields.yaml Outdated Show resolved Hide resolved
ontology/yaml/resources/fields/telemetry_fields.yaml Outdated Show resolved Hide resolved
ontology/yaml/resources/fields/telemetry_fields.yaml Outdated Show resolved Hide resolved
ontology/yaml/resources/fields/telemetry_fields.yaml Outdated Show resolved Hide resolved
@tasodorff
Copy link
Collaborator

@josephedwardchang could you please check on this PR? a few ontology validator failures due to the apparent_power adjustment, and a few comments that you mention are resolved but i still see in their previous form. please push your latest updates and pass the validator and we can get this wrapped up.

@charbull
Copy link
Collaborator

charbull commented Jan 6, 2023

@tasodorff @josephedwardchang can you please resolve the conflicts

ontology/yaml/resources/HVAC/entity_types/FCU.yaml Outdated Show resolved Hide resolved
ontology/yaml/resources/HVAC/entity_types/FCU.yaml Outdated Show resolved Hide resolved
ontology/yaml/resources/METERS/entity_types/WM.yaml Outdated Show resolved Hide resolved
- FSS
- FA2X
uses:
- failed_panel_alarm
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest using master_alarm to model this catch-all alarm

- high_zone_air_h2_concentration_alarm:
- ACTIVE
- INACTIVE
- gas_release_status:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a status (feedback) or should it be a command ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is status, ma'am, the BMS has no controls for all fire systems (just alarms). the fire controls are automated, split apart from the BMS.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it correct to call this a gas_release_valve_status ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes ma'am possibly better. I'll update now

- damper_status
- damper_command

ASD:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok that makes sense. I Notice here this type only has opt_uses (which is not best-practice). In this case, I suggest deleting the ASD type completely and simply defining these fields on the FACP_ASD type directly

@josephedwardchang
Copy link
Contributor Author

josephedwardchang commented Sep 22, 2023

Hi All,

We have pending validation on the site and we would like to get this pull request to be merged as the project needs to be closed and I'm afraid this merge is required.

Thanks

- high_zone_air_h2_concentration_alarm:
- ACTIVE
- INACTIVE
- gas_release_status:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it correct to call this a gas_release_valve_status ?

ontology/yaml/resources/fields/telemetry_fields.yaml Outdated Show resolved Hide resolved
ontology/yaml/resources/fields/telemetry_fields.yaml Outdated Show resolved Hide resolved
@cstirdivant cstirdivant dismissed tasodorff’s stale review October 4, 2023 22:11

requested changes have been addressed

@cstirdivant cstirdivant merged commit 3e4c5b4 into google:master Oct 4, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 High priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants