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.

Sign up for GitHub

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

Jump to bottom

[yeelight] Add basic support for yeelight desklamp #4453

Merged
merged 2 commits into from
Jan 28, 2019
Merged

[yeelight] Add basic support for yeelight desklamp #4453

merged 2 commits into from
Jan 28, 2019

Conversation

sebastianrakel
Copy link

@sebastianrakel sebastianrakel commented Dec 26, 2018 •
edited by wborn
Loading

This PR should provide basic functions of the desklamp, like the ceeling light

I have a desklamp at home and want to include it into my openhab environment

Fixes #4452

@cweitkamp cweitkamp added the enhancement An enhancement or new feature for an existing add-on label Dec 26, 2018
cweitkamp
cweitkamp requested changes Jan 23, 2019
View reviewed changes
Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

Thank you very much. Can you please update all headers to EPL-2.0? Thanks.

....yeelight/src/main/java/org/openhab/binding/yeelight/internal/lib/device/DesklampDevice.java
mDeviceStatus.setCt(status.get(3).getAsInt());
}
}
} catch (Exception e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know if we can catch a more specific exception type?

Copy link
Author

Choose a reason for hiding this comment

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

Maybe, i only copied a other light and changed the needed lines.

I will have a look into that at the weekend

Copy link
Author

Choose a reason for hiding this comment

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

I don't know how to catch this exception more specific, normaly this exception should not be thrown. I can remove this exception but i don't know if that is a better solution

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind. It seems to be the way how this binding handles it. Let's do not touch it though.

....yeelight/src/main/java/org/openhab/binding/yeelight/internal/lib/device/DesklampDevice.java Outdated
@@ -0,0 +1,73 @@
/**
* Copyright (c) 2010-2018 by the respective copyright holders.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also update all headers to EPL-2.0, see also the community topic.

Copy link
Author

Choose a reason for hiding this comment

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

All headers in this binding or only at the new files?

Copy link
Contributor

Choose a reason for hiding this comment

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

The header of this file is sufficient.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, sorry. Will change it at the weekend

@sebastianrakel sebastianrakel force-pushed the 4452-add-desklamp branch 2 times, most recently from c16c40e to 161ddd8 Compare January 27, 2019 20:41
@cweitkamp
Copy link
Contributor

Thanks for the updates. Can you have a look why Travis does not like the latest changes?

[ERROR] Failed to execute goal org.eclipse.tycho:tycho-compiler-plugin:1.2.0:compile (default-compile) on project org.openhab.binding.yeelight: Compilation failure: Compilation failure: 
[ERROR] /home/travis/build/openhab/openhab2-addons/addons/binding/org.openhab.binding.yeelight/src/main/java/org/openhab/binding/yeelight/internal/lib/services/DeviceManager.java:[193] 
[ERROR] 	} catch (IOException e) {
[ERROR] 	         ^^^^^^^^^^^
[ERROR] IOException cannot be resolved to a type
[ERROR] 1 problem (1 error)

@sebastianrakel
Add basic support for yeelight desklamp
d0595a1 
Issue 4452

Signed-off-by: Sebastian Rakel <[email protected]>
@sebastianrakel
Update license
4209e32 
Signed-off-by: Sebastian Rakel <[email protected]>
@sebastianrakel
Copy link
Author

@cweitkamp thanks. is fixed now

cweitkamp
cweitkamp approved these changes Jan 28, 2019
View reviewed changes
Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you too.

@cweitkamp cweitkamp merged commit b83b6ae into openhab:master Jan 28, 2019
@sebastianrakel sebastianrakel deleted the 4452-add-desklamp branch January 28, 2019 09:37
@wborn wborn modified the milestones: 2.4, 2.5 Feb 28, 2019
Pshatsillo pushed a commit to Pshatsillo/openhab-addons that referenced this pull request Jun 19, 2019
@sebastianrakel @Pshatsillo
[yeelight] Add basic support for yeelight desklamp (openhab#4453)
4ab858c 
* Add basic support for yeelight desklamp

Fixes openhab#4452

Signed-off-by: Sebastian Rakel <[email protected]>
Signed-off-by: Pshatsillo <[email protected]>
ne0h pushed a commit to ne0h/openhab-addons that referenced this pull request Sep 15, 2019
@sebastianrakel @ne0h
[yeelight] Add basic support for yeelight desklamp (openhab#4453)
a764aac 
* Add basic support for yeelight desklamp

Fixes openhab#4452

Signed-off-by: Sebastian Rakel <[email protected]>
Signed-off-by: Maximilian Hess <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cweitkamp cweitkamp cweitkamp approved these changes

Assignees
No one assigned
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
2.5
Development

Successfully merging this pull request may close these issues.
3 participants
@sebastianrakel @cweitkamp @wborn