Skip to content

Conversation

@tiyash-basu-frequenz
Copy link
Contributor

This commit adds the ElectricalComponentError message to the electrical_components.proto file. This message is designed to represent error or warning conditions reported by microgrid electrical components. It extends a standardized error code with contextual information useful for diagnostics or vendor-specific insights.

@tiyash-basu-frequenz tiyash-basu-frequenz self-assigned this Apr 15, 2025
Copilot AI review requested due to automatic review settings April 15, 2025 08:10
@tiyash-basu-frequenz tiyash-basu-frequenz requested a review from a team as a code owner April 15, 2025 08:10
@tiyash-basu-frequenz tiyash-basu-frequenz added this to the v0.7.0 milestone Apr 15, 2025
@github-actions github-actions bot added the part:protobuf Affects the protocol buffer definition files label Apr 15, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

proto/frequenz/api/common/v1/microgrid/electrical_components/electrical_components.proto:505

  • The warnings field type has been changed from ElectricalComponentErrorCode to ElectricalComponentError. Please verify that all downstream implementations consuming this field are updated accordingly to handle the extended error information.
// List of non-critical warnings detected for the component.

proto/frequenz/api/common/v1/microgrid/electrical_components/electrical_components.proto:516

  • The errors field type has been updated to ElectricalComponentError. Ensure that consumers expecting only error codes are capable of processing the additional error context provided by the new message schema.
//     No duplicate error codes allowed.

@github-actions github-actions bot added the part:docs Affects the documentation label Apr 15, 2025
Copy link
Contributor

@thomas-nicolai-frequenz thomas-nicolai-frequenz left a comment

Choose a reason for hiding this comment

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

LGTM!

@llucax
Copy link
Contributor

llucax commented Apr 15, 2025

Probably for another PR, but maybe it can be renamed to ElectricalComponentIssue to be less confusing when used as a warning?

@tiyash-basu-frequenz
Copy link
Contributor Author

Probably for another PR, but maybe it can be renamed to ElectricalComponentIssue to be less confusing when used as a warning?

Sure. I have a few more contenders:

  • ElectricalComponentAnomaly
  • ElectricalComponentDiagnostic

Also, I would also extend this to the enum name as well.

Will create an issue depending upon how the discussions in this PR go.

@ktickner
Copy link

I prefer issue or diagnostic.
Anomaly makes me think Cthulu

This commit adds the `ElectricalComponentError` message to the
`electrical_components.proto` file. This message is designed to
represent error or warning conditions reported by microgrid electrical
components. It extends a standardized error code with contextual
information useful for diagnostics or vendor-specific insights.

Signed-off-by: Tiyash Basu <[email protected]>
@thomas-nicolai-frequenz
Copy link
Contributor

ElectricalComponentDiagnostic

Would align well with other APIs.

Copy link
Contributor

@thomas-nicolai-frequenz thomas-nicolai-frequenz left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@ktickner ktickner left a comment

Choose a reason for hiding this comment

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

LGTM

@tiyash-basu-frequenz tiyash-basu-frequenz added this pull request to the merge queue Apr 15, 2025
Merged via the queue into frequenz-floss:v0.x.x with commit e2e13b3 Apr 15, 2025
6 checks passed
@tiyash-basu-frequenz tiyash-basu-frequenz deleted the component_status_update branch April 15, 2025 12:34
github-merge-queue bot pushed a commit that referenced this pull request Apr 15, 2025
…309)

This commit improves the naming of the `ElectricalComponentError`
message to `ElectricalComponentDiagnostic`, to better reflect its
purpose of representing both warnings and errors in microgrid electrical
components.

Followed up from this comment:
#308 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:docs Affects the documentation part:protobuf Affects the protocol buffer definition files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants