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

Format fixes in light module #1874

Merged
merged 5 commits into from
Aug 30, 2019
Merged

Format fixes in light module #1874

merged 5 commits into from
Aug 30, 2019

Conversation

mcspr
Copy link
Collaborator

@mcspr mcspr commented Aug 26, 2019

  • configurable max and min values
  • fix printf format specifiers with double inputs
  • fix map() with older cores, add warning
  • maybe fix hsv conversion? HSV command is not HSV set #1864

double b = (double) ((target ? _light_channel[2].target : _light_channel[2].inputValue) * brightness) / 255.0;
double r = static_cast<double>(target ? _light_channel[0].target : _light_channel[0].value);
double g = static_cast<double>(target ? _light_channel[1].target : _light_channel[1].value);
double b = static_cast<double>(target ? _light_channel[2].target : _light_channel[2].value);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I noticed this strange calculation earlier. inputValue is value without brightness, so maybe it makes sense in that case. But it tries adjust it even with target which is set from value, thus displayed value could be wrong.
@CrazyIvan359 does checking for target flag makes any sense here at all? Web sends it with false, but mqtt reports with true... I am not quite sure what was the reason for this conditional.

Copy link
Contributor

Choose a reason for hiding this comment

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

inputValue is value without brightness

This is why HSV commands resulted in the wrong RGB values, because inputValue was calculated from input HSV, then scaled by V.

I don't know what target is. I'm not particularly familiar with the codebase and my C is a bit rusty. I will have a look and test this out though, hopefully one night this week, Saturday at the latest. Thank you!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK! Thanks

bool target param for _toHSV(output, output_size, >> target <<) function, selecting channel.target aka value+brightness via ternary 'if' above.

Last change to this was 19d0c5d, which states to report target (adjusted) values to mqtt & web, but that only happened with mqtt (see lightColor(false) usage).
Maybe there's no need for conditional at all and it should always use adjusted value

@mcspr mcspr merged commit b3600bd into xoseperez:dev Aug 30, 2019
@mcspr mcspr deleted the format-fixes branch August 30, 2019 03:34
@mcspr
Copy link
Collaborator Author

mcspr commented Aug 30, 2019

Adding for #1763, to not forget to use lightMap(...)

This was referenced Aug 30, 2019
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.

None yet

2 participants