Skip to content

Correct height for PhatSSD1608 and do minor refactoring #40

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.

Sign up for GitHub

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

Open
mnishiguchi wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Correct height for PhatSSD1608 and do minor refactoring #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions lib/display/display.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,10 @@ defmodule Inky.Display do
def spec_for(type, accent \\ :black)

def spec_for(type = :phat_ssd1608, accent) do
# Keep it minimal. Details are specified in `Inky.HAL.PhatSSD1608`.
%__MODULE__{
type: type,
width: 250,
height: 122,
height: 136,

@nyaray nyaray Nov 9, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see corrections :)!

@mnishiguchi mnishiguchi Nov 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

inky-name-badge 20211109_073337

@nyaray nyaray Nov 9, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@mnishiguchi mnishiguchi Nov 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

packed_dimensions: %{},
rotation: -90,
accent: accent,
Expand Down
22 changes: 11 additions & 11 deletions lib/hal/hal_ssd1608.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ defmodule Inky.HAL.PhatSSD1608 do
@color_map_black %{black: 0, miss: 1}
@color_map_accent %{red: 1, yellow: 1, accent: 1, miss: 0}

@cols 136
@rows 250
@rotation -90
@lut_data <<0x02, 0x02, 0x01, 0x11, 0x12, 0x12, 0x22, 0x22, 0x66, 0x69, 0x69, 0x59, 0x58, 0x99,
0x99, 0x88, 0x00, 0x00, 0x00, 0x00, 0xF8, 0xB4, 0x13, 0x51, 0x35, 0x51, 0x51, 0x19,
0x01, 0x00>>
Expand Down Expand Up @@ -71,14 +68,15 @@ defmodule Inky.HAL.PhatSSD1608 do

@impl Inky.HAL
def handle_update(pixels, border, push_policy, state = %State{}) do
black_bits = PixelUtil.pixels_to_bits(pixels, @rows, @cols, @rotation, @color_map_black)
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)

@nyaray nyaray Nov 9, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 |> set_reset(0) |> sleep(500) |> set_reset(1) |> sleep(500)
state |> write_command(@cmd_soft_reset) |> sleep(1000)

case pre_update(state, push_policy) do
:cont -> do_update(state, state.display, border, black_bits, accent_bits)
:cont -> do_update(state, border, black_bits, accent_bits)
:halt -> {:error, :device_busy}
end
end
Expand All @@ -100,15 +98,17 @@ defmodule Inky.HAL.PhatSSD1608 do
end
end

@spec do_update(State.t(), Inky.Display.t(), atom(), binary(), binary()) :: :ok
defp do_update(state, _display, border, black_bits, accent_bits) do
@spec do_update(State.t(), atom(), binary(), binary()) :: :ok
defp do_update(state, border, black_bits, accent_bits) do
%{width: width, height: height} = state.display

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])

@nyaray nyaray Nov 9, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible axes swap, see my earlier comment.

|> write_command(@cmd_set_dummy_line_period, [0x1B])
|> write_command(@cmd_set_gate_line_width, [0x0B])
|> write_command(@cmd_set_data_entry_mode, [0x03])
|> 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])

@nyaray nyaray Nov 9, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible axes swap, see my earlier comment.

|> write_command(@cmd_write_vcom, [0x70])
|> write_command(@cmd_write_lut, @lut_data)
|> set_border_color(border)
Expand Down