Correct height for PhatSSD1608 and do minor refactoring #40
Conversation
| %__MODULE__{ | ||
| type: type, | ||
| width: 250, | ||
| height: 122, | ||
| height: 136, |
There was a problem hiding this comment.
Nice to see corrections :)!
There was a problem hiding this comment.
That magic number 136 was not explained in the Python library. The actual dimension is 250*122 in their catalog. Possibly it is because 122 is not divisible by 8.
So far my pHAT SSD1608 is working well. Before this PR, the image is slightly off center.
There was a problem hiding this comment.
Ahhh, so if the height is 122, the image gets cropped on the right side in the photo above, right? I guess this is similar to how iPhones recently have gotten a notch, except that it's not in the centre-top here, but instead in the top-right/bottom/left (depending on orientation). By the way, did you add support for this screen in a separate PR? If so, did you follow the flow of data/constants internally to track down where width/height might be used during setup? It would be good to verify that hardware initialisation isn't changed from how the python code does it.
The code should probably expose 122 as the height, but internally adjust things, to offset the pixels so that the entire image is shown. If the screen is meant to have 250x122 addressable pixels, I think it's best if we act as if that were the case. What do you think?
For fun, we could have an extended mode that allows you to use the extra pixels in the corner, but that is probably something for a separate, later, PR, hah.
There was a problem hiding this comment.
Top or bottom might be cropped. Yeah the Python library uses an image processing library to do some adjustment, but I am not familiar with all the image processing.
Sidenote: In my fork, I make the display up side down so it is consistent with manufacturer's example code.
https://github.com/pimoroni/inky/blob/fc17026df35447c1147e9bfa38988e89e75c80e6/examples/name-badge.py#L66
4342aeb
to
2b4f9b6
Compare
| accent_bits = PixelUtil.pixels_to_bits(pixels, @rows, @cols, @rotation, @color_map_accent) | ||
| %{width: width, height: height, rotation: rotation} = state.display | ||
| black_bits = PixelUtil.pixels_to_bits(pixels, width, height, rotation, @color_map_black) | ||
| accent_bits = PixelUtil.pixels_to_bits(pixels, width, height, rotation, @color_map_accent) |
There was a problem hiding this comment.
Why did rows become width here? This means that the axes have been swapped, was that done on purpose? This will break code that is built on the current dimensions.
| state | ||
| |> write_command(@cmd_set_driver_output, [@rows - 1, (@rows - 1) >>> 8, 0x00]) | ||
| |> write_command(@cmd_set_driver_output, [width - 1, (width - 1) >>> 8, 0x00]) |
There was a problem hiding this comment.
Possible axes swap, see my earlier comment.
| |> write_command(@cmd_set_ram_x_position, [0x00, div(@cols, 8) - 1]) | ||
| |> write_command(@cmd_set_ram_y_position, [0x00, 0x00, @rows - 1, (@rows - 1) >>> 8]) | ||
| |> write_command(@cmd_set_ram_x_position, [0x00, div(height, 8) - 1]) | ||
| |> write_command(@cmd_set_ram_y_position, [0x00, 0x00, width - 1, (width - 1) >>> 8]) |
There was a problem hiding this comment.
Possible axes swap, see my earlier comment.
The height value was wrong in one location for SSD1608.
In the Python library, there are two sets of dimensions for SSD1608, which is confusing. but it seems like
250x136works better in our code than250x122.Also I moved the display info from
Inky.HAL.PhatSSD1608toInky.Displaystruct. I think it makes SSD1608 code slightly more consistent with the other code.Notes
PILandnumpyto offset the image here but I do not know what is the equivalent in Elixir.