Simplify constructor use and documentation improvement#20
Open
drepper wants to merge 2 commits into
Open
Conversation
Except for non-standard devices, the address value and the display size change in unisom. There are exactly two pairs of these values and therefore requiring the caller to specify both the address and size in the construtor means more work, more possible errors, and knowledge on the side of the user of the library which can be avoided to be hardcoded. This patch introduces a second constructor which takes only the size parameter and derives the address parameter.
In the example code in the readme.md file, the code shows to use the initialization of the display variable with the help of a copy constructor. This is not necessary, just call the constructor directly.
digiexchris
reviewed
Feb 5, 2025
| /// \brief SSD1306 constructor with implied standard address. | ||
| /// \param i2CInst - i2c instance. Either i2c0 or i2c1 | ||
| /// \param size - display size. Acceptable values W128xH32 or W128xH64 | ||
| SSD1306(i2c_inst *i2CInst, Size size) : SSD1306(i2CInst, size == Size::W128xH32 ? 0x3C : 0x3D, size) { } |
There was a problem hiding this comment.
I this might not be a valid assumption. Most of my 128x64's are 0x3C, one is not, dependant on brand. It might be confusing to users if they see this assuming it will choose a sane default for them when such a default is entirely model dependant.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The SSD1306 constructor should not require duplicated information and bleeding of details (i.e., the I²C address) into the application code.
The documentation shows an inefficient use to construct the SSD1306 object. A minimal change does away with that and removes the superfluous double use of the full type name (alternative: use
auto).