Skip to content

Enhance pixel color detection in SensorsBlocks.js with improved error handling and additional utility methods#4545

Merged
walterbender merged 11 commits intosugarlabs:masterfrom
Ashrafmuhmed:RefactoringSensorsBlocks
Mar 22, 2025
Merged

Enhance pixel color detection in SensorsBlocks.js with improved error handling and additional utility methods#4545
walterbender merged 11 commits intosugarlabs:masterfrom
Ashrafmuhmed:RefactoringSensorsBlocks

Conversation

@Ashrafmuhmed
Copy link
Contributor

Description

This PR refactors the GetColorPixelBlock class in js/blocks/SensorsBlocks.js to improve its readability, robustness, and maintainability. These changes enhance the block’s color detection functionality and lay a foundation for future extensions.

Changes Made

  • Modularized Logic: Split the arg method into helper functions (getTurtleSafely, getPixelData, detectColor, getBackgroundColor, getFallbackColor) for better readability and testability.
  • Added Error Handling: Introduced a try-catch block to handle failures (e.g., missing turtle or canvas context), with a fallback to a default gray color (searchColors(128, 128, 128)).
  • Preserved Functionality: Kept the original behavior (color detection via canvas pixel data) while ensuring the turtle’s visibility is safely toggled.
  • Updated Documentation: Enhanced JSDoc comments to clarify return values, exceptions, and method purposes.

@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@walterbender
Copy link
Member

I think these changes are good but a couple of questions.
(1) It seems like the getTurtleSafely method is more general than just for this code block. Maybe it should be a function in turtles.js that can be reused elsewhere?
(2) We have a test suite for Sensors. Can you modify the tests to include your new methods?

@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@Ashrafmuhmed
Copy link
Contributor Author

@walterbender
(1) - I have enhanced getTurtle() to handle missing turtles.
(2) - I have added tests for the new methods in SensorsBlocks.js and all the tests pass!
Let me know if there’s anything else you’d like me to adjust!

Screenshot 2025-03-22 220314

image

@walterbender walterbender merged commit 893603a into sugarlabs:master Mar 22, 2025
5 checks passed
sa-fw-an pushed a commit to sa-fw-an/musicblocks that referenced this pull request Apr 29, 2025
… handling and additional utility methods (sugarlabs#4545)

* fix localization strings for motion-y and get-protein in Arabic

* Refactor text cleaning and translation retrieval in utils.js

* Fix comment formatting in getTranslationWithCase function

* Enhance pixel color detection in SensorsBlocks.js with improved error handling and additional utility methods

* fix Eslint warnings

* Add tests for the new methods in SensorsBlocks.js

* fix eslint warning
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.

2 participants