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
Add optional fields to set custom min/max SOC in UDI Events #325
Add optional fields to set custom min/max SOC in UDI Events #325
Conversation
…d test Signed-off-by: F.N. Claessen <felix@seita.nl>
@@ -120,6 +121,8 @@ def post_udi_event(): | |||
"datetime": "2015-06-02T16:00:00+00:00" | |||
} | |||
], | |||
"soc_min": 10, |
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.
I might misunderstand the two ways that value
can be sent in here (one as a measurement, one as a target, right?)
It is kind of confusing what the "event" is, especially when both types of value
are being sent at once.
Anyway ― Isn't it cleaner if we require these SOC constraints to be sent in within the targets
dict?
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.
The value
in the top level of the JSON denotes an SOC measurement (used as the starting value for computing the schedule). The value
in the list of targets
denotes a wish list of future values. Note that targets
is a list rather than a dict, so targets
can contain multiple values.
The soc_min
and soc_max
are not part of the "targets" wish list, but rather fixed constraints.
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 it makes sense to leave it where it is.
Maybe we should rename the measurement-value to start-value
? Of course, if this endpoint is used to only send that measurement, it does not make sense. For our future API design, I have this feeling that this should be two very clearly-defined endpoints.
@@ -120,6 +121,8 @@ def post_udi_event(): | |||
"datetime": "2015-06-02T16:00:00+00:00" | |||
} | |||
], | |||
"soc_min": 10, |
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 it makes sense to leave it where it is.
Maybe we should rename the measurement-value to start-value
? Of course, if this endpoint is used to only send that measurement, it does not make sense. For our future API design, I have this feeling that this should be two very clearly-defined endpoints.
I understand the feeling, but I want to add some considerations. I think there's also a possible misunderstanding. The feeling: One feels like posting data to a sensor, while the other feels like actuating a scheduling model. It's that what you mean? My first consideration: What I like about our current endpoint is that you can communicate (almost) the entire flexibility model within a single message. I write "almost" because prices aren't part of the message, and neither are the power constraints (yet; I do want to add power constraints to the message at some point, like we do with the SOC constraints in this PR). If we take sending the current SOC out of the UDI Event, the flexibility model will become more incomplete. My second consideration is that the SOC is not a Charge Point sensor, but an EV sensor. If we take sending the current SOC out of the UDI Event, we'll have to start modeling EV assets and their relationship with Charge Points, which isn't trivial. Finally, the possible misunderstanding is that if you take out the |
Plus test revision and changelog entry.
With this PR, the functionality of our battery and Charge Point schedulers have grown to become almost indistinguishable. I'd like to keep the scope of this PR limited to just adding the min/max SOC functionality, and I suggest I make a separate PR for refactoring the two schedulers.
Closes #244