Skip to content

Change RPCs#153

Merged
tiyash-basu-frequenz merged 4 commits intofrequenz-floss:v0.x.xfrom
tiyash-basu-frequenz:rename_rpc
Oct 6, 2023
Merged

Change RPCs#153
tiyash-basu-frequenz merged 4 commits intofrequenz-floss:v0.x.xfrom
tiyash-basu-frequenz:rename_rpc

Conversation

@tiyash-basu-frequenz
Copy link
Copy Markdown
Contributor

This PR includes the following changes:

  • Remove RPC HotStandby
  • Rename RPC ColdStandby to Standby
  • Rename RPC SubscribeComponentData to ReceiveComponentDataStream
  • Rename RPC SubscribeSensorData to ReceiveSensorDataStream

@tiyash-basu-frequenz tiyash-basu-frequenz requested a review from a team as a code owner October 5, 2023 15:10
@tiyash-basu-frequenz tiyash-basu-frequenz self-assigned this Oct 5, 2023
@tiyash-basu-frequenz tiyash-basu-frequenz linked an issue Oct 5, 2023 that may be closed by this pull request
@github-actions github-actions Bot added part:docs Affects the documentation part:protobuf Affects the protocol buffer definition files labels Oct 5, 2023
// If any of the above mentioned actions for a given component has already
// been performed, then this method call efffectively skips that action.
rpc ColdStandbyComponent(ColdStandbyComponentRequest)
rpc StandbyComponent(StandbyComponentRequest)
Copy link
Copy Markdown

@thomas-nicolai-frequenz thomas-nicolai-frequenz Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds strange. We are putting a component into standby. So maybe something EnableComponentStandby or something similiar? Here are some suggestions:

  • ToggleComponentStandbyMode

  • SetComponentToStandby

  • EnableStandbyForComponent

  • SwitchToStandbyMode

  • PutComponentInStandby

  • ActivateStandbyMode / DeactivateStandbyMode

  • ActivateComponentStandbyMode / DeactivateComponentStandbyMode

  • ToggleStandbyState

  • ChangeToStandbyMode

My question would be if a user puts an inverter into standby how can he activate it again? I think this we might also want to make clear in the documentation.

Copy link
Copy Markdown

@thomas-nicolai-frequenz thomas-nicolai-frequenz Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference would be to use a verb at the front. SetComponentToStandby or ActivateComponentStandbyMode would be my favourites.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PutComponentInStandby sounds good. I have updated the new RPC name to PutComponentInStandby.

My question would be if a user puts an inverter into standby how can he activate it again? I think this we might also want to make clear in the documentation.

Good call. I added it to the RPC docs.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StandbyComponent also sounded a bit weird to me at first, but I think it had a nicer symmetry with StartComponent and StopComponent, IMHO now it is harder to see those 3 methods are related. As a final suggestion, I would put PauseComponent to the table, but I don't have any major complains if you prefer to keep PutComponentInStandby.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(just unresolved so my last comment is not lost)

Also, not sure if it would be worth clarifying in SetComponentPowerActive docs that this will start the component if it was in standby/paused.

Signed-off-by: Tiyash Basu <tiyash.basu@frequenz.com>
The `HotStandby` RPC has been removed from the `Microgrid` service. This
RPC is used only for inverters, and most inverters do not support this
functionality. If required, the effect resulting by calling this RPC can
be achieved internally upon receiving a charge/discharge command with
power 0.

Signed-off-by: Tiyash Basu <tiyash.basu@frequenz.com>
Signed-off-by: Tiyash Basu <tiyash.basu@frequenz.com>
Signed-off-by: Tiyash Basu <tiyash.basu@frequenz.com>
@tiyash-basu-frequenz tiyash-basu-frequenz added this pull request to the merge queue Oct 6, 2023
Merged via the queue into frequenz-floss:v0.x.x with commit aa7a160 Oct 6, 2023
@tiyash-basu-frequenz tiyash-basu-frequenz deleted the rename_rpc branch October 6, 2023 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:docs Affects the documentation part:protobuf Affects the protocol buffer definition files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace HotStandby and ColdStandby with Standby

4 participants