Skip to content

Add new sensor category and metric enums#93

Merged
tiyash-basu-frequenz merged 1 commit intofrequenz-floss:v0.x.xfrom
tiyash-basu-frequenz:sensors
Sep 27, 2023
Merged

Add new sensor category and metric enums#93
tiyash-basu-frequenz merged 1 commit intofrequenz-floss:v0.x.xfrom
tiyash-basu-frequenz:sensors

Conversation

@tiyash-basu-frequenz
Copy link
Copy Markdown
Contributor

@tiyash-basu-frequenz tiyash-basu-frequenz commented Sep 26, 2023

In our terminology, components are the devices that are connected to a microgrid's electrical circuit, and sensors are the devices that measure the physical parameters of the environment.

Hence, we need to distinguish between the two, and we need to be able to identify the type of sensor and the type of metric that it measures.

This commit adds the frequenz.api.common.v1.sensors package, which contains the SensorCategory and SensorMetric enums.

Also, this commit removes

  • the COMPONENT_CATEGORY_SENSOR variant from the ComponentCategory enum, and
  • The SensorType enum.

@tiyash-basu-frequenz tiyash-basu-frequenz added this to the v0.4.0 milestone Sep 26, 2023
@tiyash-basu-frequenz tiyash-basu-frequenz self-assigned this Sep 26, 2023
@tiyash-basu-frequenz tiyash-basu-frequenz requested a review from a team as a code owner September 26, 2023 14:55
@tiyash-basu-frequenz tiyash-basu-frequenz linked an issue Sep 26, 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 Sep 26, 2023
@tiyash-basu-frequenz tiyash-basu-frequenz removed this from the v0.4.0 milestone Sep 26, 2023
Copy link
Copy Markdown
Contributor

@llucax llucax left a comment

Choose a reason for hiding this comment

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

LGTM, but it is missing the py/frequenz/api/common/v1/sensors/__init__.py (I assume this will be rebased on top of the PR adding the v1 :) ). This is just adding one file, I was confused by the text in the PR description "This commit adds the frequenz.api.common.sensors package".

llucax
llucax previously approved these changes Sep 27, 2023
In our terminology, `components` are the devices that are connected to
a microgrid's electrical circuit, and `sensors` are the devices that
measure the physical parameters of the environment.

Hence, we need to distinguish between the two, and we need to be able
to identify the type of sensor and the type of metric that it measures.

This commit adds the `frequenz.api.common,v1.sensors` package, which
contains the `SensorCategory` and `SensorMetric` enums.

Also, this commit removes
* the `COMPONENT_CATEGORY_SENSOR` variant from the `ComponentCategory`
  enum, and
* The `SensorType` enum.

Signed-off-by: Tiyash Basu <tiyash.basu@frequenz.com>
@tiyash-basu-frequenz
Copy link
Copy Markdown
Contributor Author

I was confused by the text in the PR description "This commit adds the frequenz.api.common.sensors package".

Good point. Fixed.

Copy link
Copy Markdown
Contributor

@llucax llucax left a comment

Choose a reason for hiding this comment

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

Mmm, I still see package everywhere, but I'm not sure if this is even wrong in the protobuf world (in Python packages are like directories, that contain modules, which are files). So approving because I don't think it is important really.

@tiyash-basu-frequenz tiyash-basu-frequenz added this pull request to the merge queue Sep 27, 2023
@tiyash-basu-frequenz
Copy link
Copy Markdown
Contributor Author

I assume you mean how we declare package names in protobuf files?

Declaring packages is nice, because in rust it directly maps to a module. It helps provide clean and predictable namespaces. Note that the package keyword is ignored in python.

Merged via the queue into frequenz-floss:v0.x.x with commit 9d75ad7 Sep 27, 2023
@tiyash-basu-frequenz tiyash-basu-frequenz deleted the sensors branch September 27, 2023 10: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.

Represent sensors as as their own category

2 participants