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

Add PV panel equipment definition #8

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

DougPlumley
Copy link

Hi,

@baricker and I worked on this with the team to come up with a simple definition for a PV panel, see #7.

This is our first exercise working with metadata tagging and we recognize this definition isn't fully fleshed out, so please comment/update as needed.

Thanks!

Copy link
Contributor

@annieDehghani annieDehghani left a comment

Choose a reason for hiding this comment

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

Looks like you're off to a good start! I added a few suggestions to help with conformance to our typical standards. Take a look and let me know if there are any questions.

Comment on lines +548 to +555
PVP:
name: Photovoltaic Panel
description: A collection of photovoltaic cells arranged into a panel to produce electrical energy.
brick_ontology: Photovoltaic Panel
attributes:
- ratedVoltage
- ratedCurrent
- ratedWattage
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
PVP:
name: Photovoltaic Panel
description: A collection of photovoltaic cells arranged into a panel to produce electrical energy.
brick_ontology: Photovoltaic Panel
attributes:
- ratedVoltage
- ratedCurrent
- ratedWattage
PVP:
name: Photovoltaic Panel
description: A collection of photovoltaic cells arranged into a panel to produce electrical energy.
brick_ontology: Photovoltaic Panel
attributes:
- ratedVoltage
- ratedCurrent
- ratedPower
haystack:
type:
- equip
- photovoltaic
- panel

In addition to having a unique OAP code as the key for each entity, we typically add a unique set of haystack tags to each entity. I suggest adding equip, photovoltaic (new tag that we are planning to use for PV-related entities) and panel.

Comment on lines +167 to +177
ratedWattage:
name: Rated Wattage
domain: electrical
haystack:
type:
- rated
- wattage
- attr
identification:
- kind: Number
unit_type: electric_wattage
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ratedWattage:
name: Rated Wattage
domain: electrical
haystack:
type:
- rated
- wattage
- attr
identification:
- kind: Number
unit_type: electric_wattage
ratedPower:
name: Rated Power
domain: electrical
haystack:
type:
- rated
- power
- attr
identification:
- kind: Number
unit_type: power

This is a great attribute to add as it comes up in many contexts beyond just PV panels. I suggest using the term "Power" to describe this attribute rather than wattage as it may have different units in different contexts. Additionally I would use the power tag since that's an existing tag in OAP. Lastly, the unit_type should align with the units types in field.units.yaml so I would use power as the unit_type here.

@DougPlumley
Copy link
Author

Hi @annieDehghani, your suggested changes look great, apologizes in the delay responding.

Thanks!

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