Skip to content

Conversation

@llucax
Copy link
Contributor

@llucax llucax commented Aug 27, 2025

This PR implements the ReceiveComponentDataStreamRequest rpc, and for that it adds wrappers for ComponentDataSamples, ComponentStateSample, and MetricSample.

This will be used to list electrical components.

Signed-off-by: Leandro Lucarella <[email protected]>
@Copilot Copilot AI review requested due to automatic review settings August 27, 2025 06:51
@llucax llucax requested review from a team as code owners August 27, 2025 06:51
@github-actions github-actions bot added part:tests Affects the unit, integration and performance (benchmarks) tests part:client Affects the client code labels Aug 27, 2025
@llucax llucax requested review from Marenz, eduardiazf and shsms August 27, 2025 06:51
@llucax llucax added the cmd:skip-release-notes It is not necessary to update release notes for this PR label Aug 27, 2025
@llucax llucax self-assigned this Aug 27, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements the ReceiveComponentDataStream RPC method, adding the ability to stream component data samples that include both metrics and state information. The implementation creates comprehensive wrapper classes for handling component data streaming.

  • Adds ComponentDataSamples, ComponentStateSample, and MetricSample wrapper classes
  • Implements protobuf conversion functions with error handling and issue reporting
  • Adds the receive_component_data_samples_stream method to the microgrid client
  • Includes comprehensive test coverage for all new functionality

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

File Description
src/frequenz/client/microgrid/_client.py Implements the main receive_component_data_samples_stream method with broadcaster management
src/frequenz/client/microgrid/component/ Adds new wrapper classes and protobuf conversion functions
src/frequenz/client/microgrid/metrics/ Extends metrics module with MetricSample class and protobuf conversion
tests/ Comprehensive test coverage for all new classes and conversion functions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@llucax
Copy link
Contributor Author

llucax commented Aug 27, 2025

@eduardiazf adding you because this is adding some wrappers for API common messages that I plan to move to client-common soon. I guess these are not relevant to the Assets API, but just FYI. We just merged another PR with wrappers that are probably more relevant to you:

Bear in mind that these are for the Common API pre-v0.8, so some we'll need to make some renames before moving to Client Common.

llucax added 3 commits August 27, 2025 09:08
Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax enabled auto-merge August 27, 2025 07:10
@llucax llucax merged commit 3dfaf06 into frequenz-floss:v0.17.x Aug 29, 2025
5 checks passed
Example:
A hybrid inverter can have a DC string for a battery and another DC string for a
PV array. The connection names could resemble, say, `dc_battery_0` and
``dc_pv_0`. A metric like DC voltage can be obtained from both connections. For
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
``dc_pv_0`. A metric like DC voltage can be obtained from both connections. For
`dc_pv_0`. A metric like DC voltage can be obtained from both connections. For


# In the protocol this is float | AggregatedMetricValue, but for live data we can't
# receive the AggregatedMetricValue, so we limit this to float for now.
value: float | AggregatedMetricValue | None
Copy link
Contributor

Choose a reason for hiding this comment

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

is the comment outdated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think that comment was for before 0.17, will fix.

@llucax llucax deleted the recv-comp-data branch September 19, 2025 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmd:skip-release-notes It is not necessary to update release notes for this PR part:client Affects the client code part:tests Affects the unit, integration and performance (benchmarks) tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants