Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

[Tradfri] Fix bug in color space conversion #4351

Merged

Conversation

hreichert
Copy link
Contributor

@hreichert hreichert commented Sep 26, 2017

Ensure that calculated RGB values are not negative.
Added new unit test case.

Fixes: #4349

@openhab-bot
Copy link
Contributor

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/ikea-tradfri-gateway/26135/127

int redRounded = (int) Math.round(red * 255.0);
int greenRounded = (int) Math.round(green * 255.0);
int blueRounded = (int) Math.round(blue * 255.0);
int redRounded = ensureRgbRange((int) Math.round(red * 255.0));
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this is the right place for the fix. Shouldn't we make sure that red/green/blue is not negative and not above 1? At least the later is already checked in lines 108-120 so you would only need to check for negative numbers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your comment, that would be indeed better.
I'm going to change this and rebase on master, as there are now duplicate method names in the tests I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Ensure that calculated RGB values are not negative.

Fixes: eclipse-archived#4349

Signed-off-by: Holger Reichert <mail@h0lger.de>
@hreichert hreichert force-pushed the 4349-tradfri-fix-color-conversion branch from 0f74e56 to d61ac8d Compare October 2, 2017 19:27
@hreichert
Copy link
Contributor Author

Updated PR with suggested change from @triller-telekom

@sjsf sjsf merged commit d93353b into eclipse-archived:master Oct 4, 2017
@sjsf
Copy link
Contributor

sjsf commented Oct 4, 2017

Thanks!

@hreichert hreichert deleted the 4349-tradfri-fix-color-conversion branch October 4, 2017 17:23
@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017
@kaikreuzer kaikreuzer added the bug label Dec 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants