Closed tiyash-basu-frequenz closed 7 months ago
@llucax @shsms thoughts?
Without being a user of the microgrid API myself, it looks like some extra flexibility might be welcome, but I don't feel involved enough to see pitfalls or other implication that might come up from this change. Maybe opinions from @daniel-zullo-frequenz, @Marenz or @idlir-shkurti-frequenz could be valuable.
I don't really see the use case so far though
I think this might be useful for peak-shaving. We can use this feature to set the duration of the power command entering the last minute of the time-window to prevent continue charging at the beginning of the next window when there is not yet data available to make predictions. Otherwise we need to send power zero at the end of each window. Anyway that was just to mention one use-case, and there may be others. I don't (fore)see any drawbacks on keeping the existent functionality while also introducing the flexibility for users to adjust it.
As a precaution, you might want to have checks in place for minimum and maximum valid-until duration to prevent users from abusing or misusing the feature.
I mean, I see the advantage, but it's not a big one, compared to just sending 0 again, question then becomes, is that worth the costs of implementing this
Actually safety (or precision) could be a case here, if PS, is lagging for some reason (or crashes), that 0 could be sent too late or not at all, with the auto-reset feature, things would still work pretty much as expected in that case.
I don't (fore)see any drawbacks on keeping the existent functionality while also introducing the flexibility for users to adjust it.
I plan to keep the current behaviour of stopping commands after 60s as the default option. (I updated te issue description to make this explicit)
As a precaution, you might want to have checks in place for minimum and maximum valid-until duration to prevent users from abusing or misusing the feature.
I added two solutions to the proposal section above. Would appreciate if you could take a look @daniel-zullo-frequenz.
I'm not so sure about doing it with Peak Shaving, because it keeps sending new power values almost until the end of the window, so we'd have to set very short command durations like one or two seconds, if we didn't have to send a zero at the end. So I think it would be hard to use this feature to avoid sending zero.
It makes it more transparent that issued commands are stopped after a finite amout of time. Users can then refer to the protocol definition, and not just the API documentation.
But this alone sounds like a good enough reason to do it.
Do any of you thoughts/opinions on the proposed solutions?
Because this is for set_power*
and will be exposed to SDK users, I like having floats, because those are easier to translate to idiomatic Python on the client side.
I personally found Solution 1
slightly better as it is open/flexible to the user/client
I understand solution 2 might be safer in a way (for cases when it is misused), but I agree solution 1 seems more natural, flexible, and easier to use when used properly.
What I like about solution 2 is that the users do not need to rely on API docs to check the limits of this config. It is expressed directly in the API. Reduces the chances of runtime errors. But this comes at the cost of flexibility.
What's needed?
Currently, the set-power commands from clients are invalidated after 60s, by the service. The API could be extended to allow clients specify their own durations for this, instead of having to always rely on the default 60s.
Allowing more choices for the valid-until time allows the following:
Note that the commands will still be invalidates by the Microgrid Service after 60s by default (if the duration is not specified in the request).
Use cases
Proposed solution
Solution 1
Update the request parameter to provide an arbitrary duration, but specify lower and upper limits of the parameter in docs.
Solution 2
Update the request parameter to provide durations from an enum.
Alternatives and workarounds
No response
Additional context
No response