-
Notifications
You must be signed in to change notification settings - Fork 219
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
feat: enable convertBitmapToLEDHex to trim or keep empty columns #1140
Conversation
This commit renames the previously unused `isDrawn` parameter to `trim` and uses it to decide whether empty columns should be removed or not.
Reviewer's Guide by SourceryThis pull request renames the Sequence diagram for bitmap to LED hex conversion with trimmingsequenceDiagram
participant Client
participant Converters
Client->>Converters: convertBitmapToLEDHex(image, trim=true)
Note over Converters: Process image from left to right
Note over Converters: If trim=true, remove empty columns
Note over Converters: Process image from right to left
Note over Converters: If trim=true, remove empty columns
Converters-->>Client: Return LED hex format strings
Class diagram showing Converters class changesclassDiagram
class Converters {
+convertBitmapToLEDHex(List~List~int~~ image, bool trim) List~String~
note for Converters "Parameter renamed from isDrawn to trim"
note for Converters "trim=true removes empty columns"
}
State diagram for bitmap processing with trim optionstateDiagram-v2
[*] --> CheckColumn
CheckColumn --> SumPixels: For each column
SumPixels --> CheckTrim: Sum = 0
CheckTrim --> MarkForRemoval: trim = true
CheckTrim --> KeepColumn: trim = false
SumPixels --> KeepColumn: Sum > 0
MarkForRemoval --> NextColumn
KeepColumn --> NextColumn
NextColumn --> CheckColumn: More columns
NextColumn --> [*]: No more columns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ZauberNerd - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please add a test case that verifies the behavior when trim=false to ensure both code paths are covered
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This commit renames the previously unused
isDrawn
parameter totrim
and uses it to decide whether empty columns should be removed or not.
Summary by Sourcery
New Features:
trim
parameter toconvertBitmapToLEDHex
to control whether empty columns are removed during conversion.