Skip to content

Conversation

@zealot-zew
Copy link

@omsuneri Refactored the redundant GetRedBlock, GetGreenBlock, and GetBlueBlock classes into a single GetColorComponentBlock base class. This reduces duplicate logic and keeps color component handling consistent. The blocks are now created dynamically using a config list.

Should we consider storing these block instances on activity for easier access or debugging?

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2025

✅ All Jest tests passed! This PR is ready to merge.

@omsuneri omsuneri self-requested a review November 9, 2025 19:42
@walterbender
Copy link
Member

This looks good. I thought we had it working with images but it seems to not work on the master branch either. @FirePheonix what do you think?

@FirePheonix
Copy link
Contributor

the detection on the master branch is not working currently for underlying images currently.
It works in Lego Bricks Widget.
I'll fix the issue soon.
(and yes, this PR does NOT conflict with Lego Bricks Color Detection)

On reviewing this PR, I realize the current detection logic on the master branch, it's returning RED every single time.

In SensorBlocks.js file on master branch, see lines: 544, 596 and 648

they're returning obj[0]/2.55
So, if RGB: (A,B,C)
then only the value corresponding to red is being returned since obj[0] is the numerator.
ALL THREE are currently returning RED component of RGB's value.

THIS PR would not just Refactor the code, BUT ALSO fix the underlying issue by returning the correct component since it returns a component for every color:

that is, for example: (component/2.55) for R,G,B each.

This is a good PR.

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