Allow targeting categories with subtypes#168
Conversation
github-actions
bot
added
part:tests
llucax
left a comment
llucax
left a comment
There was a problem hiding this comment.
LGTM in general, although not completely convinced about the current approach with CategoryAndType, looks a bit too low-level for the "high-level interface".
| class EvChargerType(IntEnum): | ||
| """Enum representing the type of EV charger.""" | ||
|
|
||
| EV_CHARGER_TYPE_UNSPECIFIED = PBEvChargerType.EV_CHARGER_TYPE_UNSPECIFIED | ||
| """Unspecified type of EV charger.""" | ||
|
|
||
| EV_CHARGER_TYPE_AC = PBEvChargerType.EV_CHARGER_TYPE_AC | ||
| """AC EV charger.""" | ||
|
|
||
| EV_CHARGER_TYPE_DC = PBEvChargerType.EV_CHARGER_TYPE_DC | ||
| """DC EV charger.""" | ||
|
|
||
| EV_CHARGER_TYPE_HYBRID = PBEvChargerType.EV_CHARGER_TYPE_HYBRID | ||
| """Hybrid EV charger.""" |
There was a problem hiding this comment.
I have these in the microgrid API too, next step is to move them to client-common so we can share it. I will do a PR with this soon after I finish the microgrid API 0.17 PR, probably this week. But just FYI, I think we should move forward adding it where needed locally as it might take some time to agree on something we are all happy about to put in common + release. In any case, I would remove the EV_CHARGER_TYPE_ prefix here.
|
|
||
| This is a frozen set, so it is immutable. | ||
| The target components are used to specify the components that a dispatch | ||
| should target. |
There was a problem hiding this comment.
Maybe include a match example here?
|
|
||
| import json | ||
| from datetime import datetime, timedelta, timezone | ||
| from sre_constants import IN |
There was a problem hiding this comment.
I bet this was Tab-oriented-programming + auto-import 😆
| from sre_constants import IN |
There was a problem hiding this comment.
it was more desperation, but this is a draft after all :P
Marenz
requested review from
llucax,
shsms and
stefan-brus-frequenz
and removed request for
shsms and
stefan-brus-frequenz
Marenz
force-pushed
the
api_update
branch
2 times, most recently
from
b5f513a to
722b878
Compare
| This is a frozen set, so it is immutable. | ||
| """ | ||
|
|
||
| def __new__(cls, *ids: SupportsInt) -> "TargetIds": |
There was a problem hiding this comment.
😱 you can't define __init__ becuase it is frozen? 🥶
There was a problem hiding this comment.
I guess? THe init ctor doesn't take parameters :)
Marenz
force-pushed
the
api_update
branch
2 times, most recently
from
53a74f2 to
4ccb8e7
Compare
Renames a few fields, but stays compatible otherwise. Signed-off-by: Mathias L. Baumann <mathias.baumann@frequenz.com>
..and support the latest CategoryAndType target type. This implements extra classes for each target component case: * List of Ids `TargetIds` * List of categories: `TargetCategories` * List of categories + subtypes: `TargetCategoriesAndTypes` This was easier to do together as the addition of the new type increased the places where it was difficult to find out what the actual target type/ids is/are. Signed-off-by: Mathias L. Baumann <mathias.baumann@frequenz.com>
|
Merging as it only had decorative / rebase changes |
3 participants
Fixes #139