Add new codes to ElectricalComponentDiagnosticCode enum
#353
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit: - Removes the diagnostic codes for `UNDERVOLTAGE_SHUTDOWN`, since it can be inferred from the `UNDERVOLTAGE` code. - Moves the `EV_UNEXPECTED_PILOT_FAILURE` code to the EV section, since it is more relevant there. - Renumbers the diagnostic codes for electrical components to ensure that they are in a logical order and do not overlap with the EV codes. Signed-off-by: Tiyash Basu <[email protected]>
github-actions
bot
added
part:docs
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR updates the ElectricalComponentDiagnosticCode enum by renaming, renumbering, and adding new diagnostic codes to cover more electrical component scenarios, especially for inverter issues.
- Removed the UNDERVOLTAGE_SHUTDOWN code in favor of UNDERVOLTAGE.
- Renumbered existing codes and added new ones for leakage current, insulation resistance, grid voltage/frequency issues, and PV/inverter faults.
- Updated the release notes to reflect these changes.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| proto/frequenz/api/common/v1/microgrid/electrical_components/electrical_components.proto | Updated diagnostic code renumbering, removal of an outdated code, and addition of new fault codes. |
| RELEASE_NOTES.md | Expanded release notes to document changes made to the ElectricalComponentDiagnosticCode enum. |
proto/frequenz/api/common/v1/microgrid/electrical_components/electrical_components.proto
Outdated
Show resolved
Hide resolved
proto/frequenz/api/common/v1/microgrid/electrical_components/electrical_components.proto
Outdated
Show resolved
Hide resolved
tiyash-basu-frequenz
force-pushed
the
231_inv_err_code
branch
from
2cda904 to
1939562
Compare
The new codes help cover more cases for inverters. Signed-off-by: Tiyash Basu <[email protected]>
tiyash-basu-frequenz
force-pushed
the
231_inv_err_code
branch
from
1939562 to
88db544
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refines the ElectricalComponentDiagnosticCode enum by removing an old undervoltage code, renumbering existing values, and adding a broad set of new diagnostic codes covering inverters, PV panels, EV components, grid faults, and more.
- Remove
UNDERVOLTAGE_SHUTDOWNand replace it with a more general undervoltage code - Renumber existing enum values to collapse gaps
- Add new codes for leakage, grid conditions, PV faults, inverter DC limits, and EV scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| proto/frequenz/api/common/v1/microgrid/electrical_components/electrical_components.proto | Removed old undervoltage shutdown code, renumbered entries, and added many new diagnostic codes |
| RELEASE_NOTES.md | Updated release notes to describe the enum changes |
Comments suppressed due to low confidence (3)
RELEASE_NOTES.md:45
- Release notes mention replacing with
UNDERVOLTAGE, but the code isn’t present. Please update the notes or add the new enum entry accordingly.
- - The code `UNDERVOLTAGE_SHUTDOWN` has been removed in favour of `UNDERVOLTAGE`.
proto/frequenz/api/common/v1/microgrid/electrical_components/electrical_components.proto:289
- Many new diagnostic codes have been added; consider adding or updating unit tests to cover serialization, parsing, and handling of each new enum value.
// There is excessive leakage current in the component.
proto/frequenz/api/common/v1/microgrid/electrical_components/electrical_components.proto:268
- The PR removes
UNDERVOLTAGE_SHUTDOWNin favor of a newUNDERVOLTAGEcode, but I don’t seeUNDERVOLTAGEdefined. Please add the generalUNDERVOLTAGEenum entry or clarify its replacement.
ELECTRICAL_COMPONENT_DIAGNOSTIC_CODE_FAULT_CURRENT = 14;
proto/frequenz/api/common/v1/microgrid/electrical_components/electrical_components.proto
Outdated
Show resolved
Hide resolved
proto/frequenz/api/common/v1/microgrid/electrical_components/electrical_components.proto
Outdated
Show resolved
Hide resolved
…lectrical_components.proto Co-authored-by: Copilot <[email protected]> Signed-off-by: thomas-nicolai-frequenz <[email protected]>
llucax
changed the title
ElectricalComponentDiagnosticCode enumElectricalComponentDiagnosticCode enum
llucax
approved these changes
May 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The following changes have been made to the
ElectricalComponentDiagnosticCodeenum (previouslyComponentErrorCode):UNDERVOLTAGE_SHUTDOWNhas been removed in favour ofUNDERVOLTAGE.closes #231