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

Support for sensor information #13

Merged
merged 1 commit into from
Dec 29, 2018
Merged

Conversation

bboehmke
Copy link
Contributor

@bboehmke bboehmke commented Apr 3, 2018

I added basic support to retrieve (and update) sensor information from a bridge.

I also tried to add a simple test for this feature.

Collinux added a commit that referenced this pull request Apr 4, 2018
sensor.go Outdated
}

/// Update sensor attributes
func (s *Sensor) Update() error {
Copy link
Owner

@Collinux Collinux Apr 4, 2018

Choose a reason for hiding this comment

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

This function name seems a bit misleading since "update" is a term used in package managers and my first thought was that it pushes a firmware update to the sensor. Perhaps "refresh" (or another related term) would be a more descriptive name for a function that re-grabs the sensor's state?

@Collinux
Copy link
Owner

Collinux commented Apr 4, 2018

At a first glance the code looks good, thank you for taking the time to contribute this!
I currently don't have a sensor in my hue system so I can't truly validate the code yet... I'll get a hold of one and check back on this. Feel free to ping me if it slips my mind.

@bboehmke
Copy link
Contributor Author

bboehmke commented Apr 4, 2018

I renamed the function to refresh.

Btw the daylight sensor is provided by the bridge itself so at least one sensor shuold be present in each bridge.

@mitchellrj mitchellrj mentioned this pull request Aug 3, 2018
@mitchellrj
Copy link

Hi, I tested this PR on my own setup and added a few extra bits in #16.

@Collinux
Copy link
Owner

Collinux commented Sep 9, 2018

I just ordered a sensor so I can test this PR and @mitchellrj 's PR.

@Collinux Collinux merged commit 6266fd8 into Collinux:master Dec 29, 2018
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.

3 participants