Added Ssd1306 Device Binding #188
Conversation
| { | ||
| byte Value { get; } | ||
|
|
||
| byte[] GetBytes(); |
There was a problem hiding this comment.
If we do end up changing the way we get the bytes of the command in order to use Span and cheaper operations, we should probably expose the Length of bytes from this interface as well.
src/devices/Ssd1306/Command/ContinuousVerticalAndHorizontalScrollSetup.cs
Outdated
Show resolved
Hide resolved
src/devices/Ssd1306/Command/SetPageStartAddressForPageAddressingMode.cs
Outdated
Show resolved
Hide resolved
| /// <summary> | ||
| /// The value that represents the command. | ||
| /// </summary> | ||
| public byte Id => (byte)ScrollType; |
There was a problem hiding this comment.
Does casting the VerticalHorizontalScrollType enum to byte give the expected value?
There was a problem hiding this comment.
I'm assuming by ContinuousVerticalAndHorizontalScrollSetupTests. Is there something else I need to be testing/checking?
|
I can, but will need to be in about 3 hours. |
| @@ -13,7 +13,7 @@ public class SetMultiplexRatio : ICommand | |||
| /// The output pads COM0-COM63 will be switched to the corresponding COM signal. | |||
| /// </summary> | |||
| /// <param name="multiplexRatio">Multiplex ratio with a range of 15-63.</param> | |||
| public SetMultiplexRatio(byte multiplexRatio = 0x63) | |||
| public SetMultiplexRatio(byte multiplexRatio = 63) | |||
There was a problem hiding this comment.
Is this something we want to expose in the public surface area? Why not just public SetMultiplexRatio(byte multiplexRatio)
There was a problem hiding this comment.
There are a few variables that have defaults when the device is reset. I thought it would be useful to have those as optional in the commands, but can remove if you think that approach is better. I'm on the fence.
There was a problem hiding this comment.
If having it explicitly be part of the API surface is useful to the caller and we don't ever expect it to change in the future, then it's fine to keep. The concern of exposing the value is that we can never change the "default" value it in the future.
There was a problem hiding this comment.
I am not a domain expert here, so take my suggestion with a grain of salt. It could very well be fine to keep the default parameter. cc @joperezr - thoughts?
|
@ahsonkhan I believe I now have all the updates in. I want to thank you again for the input/help. Stressful, but truly a learning opportunity 😄 |
|
Thanks for getting this in @shaggygi ! |
This includes the initial binding for the SSD1306 OLED display controller. It includes all commands to perform operations like configuring paging, scrolling and other display settings.
It also includes a method to send raw data. However, it is not as simple as sending "Hello .NET" as it doesn't know about text/images. There needs to be a future PR and separate graphics library created to efficiently perform the conversion to raw data to send to a binding like this. See #178.
This binding also only works with I2C devices for now. There are SPI devices available and this binding could be improved in a future PR for those.