Supply allowed string values of @AttributeDefinition channels
Until now, AttributeDefinition channels (channels, whose names start with _Property) which are based on Enums do not provide the allowed values in their type defintions retrieved via edge call getChannelsOfComponent().
For example the channel ctrlEvcs0/_PropertyChargeMode provides:
{
"id": "_PropertyChargeMode",
"accessMode": "RO",
"persistencePriority": "HIGH",
"text": "",
"type": "STRING",
"unit": "",
"category": "OPENEMS_TYPE"
},
With this pull request, the allowed enum values (which were stored internally already previously) are published correctly:
{
"id": "_PropertyChargeMode",
"accessMode": "RO",
"persistencePriority": "HIGH",
"text": "",
"type": "STRING",
"unit": "",
"category": "OPENEMS_TYPE",
"options": [
"FORCE_CHARGE",
"EXCESS_POWER"
]
},
Does this solve the Issue
Feb 20 21:56:23 server.io java[1239557]: 2025-02-20T21:56:23,788 [socket-1] WARN [nflux.FieldTypeConflictHandler] [Timedata.InfluxDB] Unable to convert field [_appManager/_PropertyApps] value ["[\n {\n \"appId\": \"App.Api.ModbusTcp.ReadOnly\",\n \"alias\": \"\",\n \"instanceId\": \"876b92bf-0fa4-4dc6-8cfc-cb0d49d3db70\",\n \"properties\": {\n \"CONTROLLER_ID\": \"ctrlApiModbusTcp0\"\n }\n },\n {\n \"appId\": \"App.Api.RestJson.ReadOnly\",\n \"alias\": \"\",\n \"instanceId\": \"5c989095-7496-4ec4-b142-7aafbf03b37c\",\n \"properties\": {\n \"CONTROLLER_ID\": \"ctrlApiRest0\"\n }\n }\n]"] to integer Feb 20 21:56:23 server.io java[1239557]: 2025-02-20T21:56:23,788 [socket-1] WARN [nflux.FieldTypeConflictHandler] [Timedata.InfluxDB] Unable to convert field [_appManager/_PropertyCurrency] value ["EUR"] to integer Feb 20 21:56:23 server.io java[1239557]: 2025-02-20T21:56:23,788 [socket-1] WARN [nflux.FieldTypeConflictHandler] [Timedata.InfluxDB] Unable to convert field [_appManager/_PropertyIgnoreStateComponents] value ["[\"\"]"] to integer Feb 20 21:56:23 server.io java[1239557]: 2025-02-20T21:56:23,788 [socket-1] WARN [nflux.FieldTypeConflictHandler] [Timedata.InfluxDB] Unable to convert field [_appManager/_PropertyKeyForFreeApps] value ["0000-0000-0000-0000"] to integer
?
No idea. I don't see any connection to this pull request.
Is there anything which I need to do in order to receive a review or other feedback about how to proceed?
Codecov Report
:x: Patch coverage is 0% with 38 lines in your changes missing coverage. Please review.
:x: Your patch check has failed because the patch coverage (0.00%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.
Additional details and impacted files
@@ Coverage Diff @@
## develop #3020 +/- ##
=============================================
+ Coverage 59.51% 59.51% +0.01%
Complexity 113 113
=============================================
Files 2765 2765
Lines 119513 119546 +33
Branches 8892 8898 +6
=============================================
+ Hits 71114 71137 +23
- Misses 45723 45733 +10
Partials 2676 2676
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Thank you for the contribution. I'll need some time to review it in detail. Ideally I would like to not add it to the parent Doc directly as it relates only to Enums. Also the code does not fully align with our coding guidlines (https://openems.github.io/openems.io/openems/latest/contribute/coding-guidelines.html)
Thank you for having a first look and taking the time for a full review. I guess with "parent doc", you refer to Doc.java? I did not find a way to modify only AbstractDoc.java. Logically, its an enum channel, but technically, it is not. (Its ChannelCategory.OPENEMS_TYPE using OpenemsType.STRING. The only difference is the schema it comes with.)
Regarding the the style warnings: During the last hour, I applied the Eclipse code formatter to all modified files and updated the branch accordingly. PR should be clean now.
@sfeilmeier : do you think you will find the time to do a full review anytime soon?
Is there any chance to get this MR merged? If not, please let me know and share a short explanation
Then I would just close it and stop investing time in doing regularly rebases and retests.
Is there any chance to get this MR merged? If not, please let me know and share a short explanation
Then I would just close it and stop investing time in doing regularly rebases and retests.
If i am honest i do not know the exact Usecase as neither the UI nor any other Component uses them incorrectly - if you would provide a little more Use-Case info it would be good i guess. I am not in the Position to Merge nor to reject - just my Questions! :)
Yes, valid and good question.
Having this information available in the channel definition will allow for a generic integration into other smart home systems.
Instead of hard-coding behaviors and options, integrations to connected systems can use the information retrieved via the GetChannelsOfComponent reponse to define possible user and system interactions.
One example of such a connected system could be Home Assistant. But this MR is fully independent of the concrete connected system.
Sorry, I hardly find time to work on this feature. The two reasons why I don't immediately accept your current implementation:
- In
EdgeConfigwe already parse Enums correctly, when creating the Schema (https://github.com/OpenEMS/openems/blob/develop/io.openems.common/src/io/openems/common/types/EdgeConfig.java#L821). This info would have to be propagated to where the Channel is created (https://github.com/OpenEMS/openems/blob/develop/io.openems.edge.common/src/io/openems/edge/common/component/AbstractOpenemsComponent.java#L306) - There are no JUnit tests to validate this new behaviour. We just had a similar issue with our API for external home automation systems which also only happend because nobody was aware a dependency update could break it (see https://github.com/OpenEMS/openems/pull/3068)