Skip to content
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 power_generation sensor #105

Merged
merged 3 commits into from
Apr 11, 2024
Merged

Conversation

cyroxx
Copy link
Contributor

@cyroxx cyroxx commented Aug 23, 2023

EDIT: We have a solar panel near our space. So far, spaceapi only allowed for power_consumption, but power_generation was still missing.
This PR intends to change that.

An example can be found at: https://spaceapi.ccc-p.org/

Copy link
Member

@rnestler rnestler left a comment

Choose a reason for hiding this comment

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

A bit more explanation and an example of such a sensor would be appreciated 🙂

15-draft.json Outdated
"type": "number"
},
"unit": {
"description": "The unit of the sensor value. You should always define the unit though if the sensor is a flag of a boolean type then you can of course omit it.",
Copy link
Member

Choose a reason for hiding this comment

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

The description does seem a bit off? How can the sensor be of boolean type? Also since it is a required field one can't omit it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I copy&pasted from power consumption and did not have a close enough look at it.
So anything that you might request here is probably also wrong at power_consumption :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have remove the misleading part of the description in 961e696.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks we'll fix this for the other sensor.

@cyroxx
Copy link
Contributor Author

cyroxx commented Sep 8, 2023

@rnestler Please review again.

@rnestler rnestler changed the title add power_generation Add power_generation sensor Apr 11, 2024
@rnestler rnestler self-requested a review April 11, 2024 19:49
@rnestler rnestler self-assigned this Apr 11, 2024
@gidsi gidsi merged commit ffd90f2 into SpaceApi:master Apr 11, 2024
8 checks passed
Comment on lines +833 to +834
"W",
"VA"
Copy link
Member

Choose a reason for hiding this comment

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

We removed mW since it doesn't make sense to have it, since 1e-3W are equal to 1mW. VA on the other hand has a slightly different meaning since it is used for Apparent power.

Copy link
Member

Choose a reason for hiding this comment

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

We'll remove it from other sensors as well.

@dbrgn dbrgn added this to the API v15 milestone Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants