Skip to content

Add function to adjust gamma#28

Merged
sajattack merged 3 commits into
sajattack:masterfrom
austinleroy:master
May 28, 2025
Merged

Add function to adjust gamma#28
sajattack merged 3 commits into
sajattack:masterfrom
austinleroy:master

Conversation

@austinleroy
Copy link
Copy Markdown

First of all, thank you for making such a straightforward driver library! Super useful and easy to see what's going on.

I made this PR because of a color issue I was seeing during use. The gamma seemed to be way out of whack, and there wasn't a way to adjust gamma or write raw commands. This new function allows for manually setting the gamma correction values, and I also provided some default positive & negative values that worked well for my device (not sure how globally applicable they are, but figured it didn't hurt).

@sajattack
Copy link
Copy Markdown
Owner

Interesting, but indeed, hardcoding values that worked on one device may not be the best approach. I wonder if software gamma correction would be too slow? Probably. Perhaps a better approach would be to move the magic numbers to documentation or example code? (ie https://github.com/sajattack/st7735-lcd-examples or a doctest ) ?

Anyways,
I'll be travelling for a week starting tomorrow, and won't have time to look at this further until I get back. I appreciate the contribution and remind me if I forget to take another look when I'm back.

@austinleroy
Copy link
Copy Markdown
Author

I pushed a quick commit moving the magic numbers from code to a comment on the adjust_gamma function. I'm open to pushing an example to the examples repo if you desire (I'm working with an ESP-WROOM-32 devboard, which I don't see in the docs already).

Although now I'm curious whether the gamma correction is needed in my case due to my TFT module or my microcontroller...

@sajattack
Copy link
Copy Markdown
Owner

I think this would be a good compromise between a comment and a full-blown example. https://doc.rust-lang.org/rustdoc/write-documentation/documentation-tests.html

@austinleroy
Copy link
Copy Markdown
Author

I could move the comment to a doc test, but ideally it would need to compile. That would necessitate either bringing in a crate with implementations for at least the SpiDevice and OutputPin traits, or mocking out those traits in code somewhere. I'll defer to you on whether you'd rather add a dev dependency, add dummy trait implementation test code, or just go with a full-blown example.

Comment thread src/lib.rs Outdated
@sajattack
Copy link
Copy Markdown
Owner

I could move the comment to a doc test, but ideally it would need to compile. That would necessitate either bringing in a crate with implementations for at least the SpiDevice and OutputPin traits, or mocking out those traits in code somewhere. I'll defer to you on whether you'd rather add a dev dependency, add dummy trait implementation test code, or just go with a full-blown example.

Fair enough.

@sajattack
Copy link
Copy Markdown
Owner

Thanks, I'm going to work on a few more changes on top of this before releasing the next version.

@sajattack sajattack merged commit 249789c into sajattack:master May 28, 2025
1 check passed
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