-
Notifications
You must be signed in to change notification settings - Fork 45
Ability to add sensor capacity if they measure power #334
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
Ability to add sensor capacity if they measure power #334
Conversation
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I don't particularly like adding this custom logic to support a single attribute, also because it's an attribute that we intend to replace by a sensor of itself. I have an alternative suggestion:
@click.option(
"--attributes",
required=False,
type=str, # a json string, actually (maybe there is out-of-the-box validation for that)
help="Additional attributes. Hint: for sensors that measures power, use {"capacity_in_mw": 10} to set a capacity of 10 MW",
default = "{}",
)
|
We have logic relying on that attribute being there, so it's not weird to also have logic to make sure the power sensors have it when being made. |
Signed-off-by: Nicolas Höning <nicolas@seita.nl>
Flix6x
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I like this better, and just as a placeholder. I'd like us to follow up with a ticket to replace any logic using the capacity_in_mw attribute, such that it becomes a completely optional attribute. I believe it's at least used to set the scale range on the latest state plot, and by schedulers to set constraints on the power flow. For the former we can use a max on the history, or let it go altogether (let the chart determine the scale range), and for the latter we can let constraints be set through a dedicated UDI event field (an optional field, in case the sensor attribute is set as a fallback).
|
I agree. I'll make the ticket. As a slightly longer outlook: For the former (and the latter maybe as well) we can also first look for a dedicated capacity sensor, then fall back to something more simple and robust. In 2022, I think we need to make sure the new data model is super straightforward to use, as well as robust. |
This is useful in general, but especially for getting started and then seeing your power sensors on the dashboard.