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

Minor fix to color XY calculation #3656

Merged
merged 1 commit into from
Jan 10, 2022
Merged

Minor fix to color XY calculation #3656

merged 1 commit into from
Jan 10, 2022

Conversation

visualage
Copy link
Contributor

The color XY uint16 representation in zigbee spec is in s15Fixed16Number format per ICC spec (https://www.color.org/specification/ICC1v43_2010-12.pdf). It is actually a 16-bit fixed-point number. If we don't consider negative numbers, it is exactly the integer divide by 65536 instead of 65535. Minor, but better to get it according to spec.

The color XY uint16 representation in zigbee spec is in `s15Fixed16Number` format per ICC spec (https://www.color.org/specification/ICC1v43_2010-12.pdf). It is actually a 16-bit fixed-point number. If we don't consider negative numbers, it is exactly the integer divide by 65536 instead of 65535. Minor, but better to get it according to spec.
@Koenkk Koenkk merged commit 4586475 into Koenkk:master Jan 10, 2022
@Koenkk
Copy link
Owner

Koenkk commented Jan 10, 2022

Thanks!

@TheJulianJES
Copy link
Contributor

It looks like this change wasn't made in all places. Shouldn't cause any major issues, but it would be nice to have it correct everywhere.

If I'm right, it should also be changed in these places (and likely others):

result.color.x = mapNumberRange(msg.data['currentX'], 0, 65535, 0, 1, 4);
}
if (msg.data.hasOwnProperty('currentY')) {
result.color.y = mapNumberRange(msg.data['currentY'], 0, 65535, 0, 1, 4);

zclData.colorx = utils.mapNumberRange(xy.x, 0, 1, 0, 65535);
zclData.colory = utils.mapNumberRange(xy.y, 0, 1, 0, 65535);

value = {x: utils.mapNumberRange(colorXY.x, 0, 1, 0, 65535), y: utils.mapNumberRange(colorXY.y, 0, 1, 0, 65535)};

const xScaled = utils.mapNumberRange(xy.x, 0, 1, 0, 65535);
const yScaled = utils.mapNumberRange(xy.y, 0, 1, 0, 65535);
extensionfieldsets.push({'clstId': 768, 'len': 4, 'extField': [xScaled, yScaled]});
state['color_mode'] = constants.colorMode[2];
state['color_temp'] = val;
} else if (attribute === 'color') {
try {
val = JSON.parse(val);
} catch (e) {
e;
}
const newColor = libColor.Color.fromConverterArg(val);
if (newColor.isXY()) {
const xScaled = utils.mapNumberRange(newColor.xy.x, 0, 1, 0, 65535);
const yScaled = utils.mapNumberRange(newColor.xy.y, 0, 1, 0, 65535);

const xScaled = utils.mapNumberRange(colorXY.x, 0, 1, 0, 65535);
const yScaled = utils.mapNumberRange(colorXY.y, 0, 1, 0, 65535);

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