Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 39 additions & 0 deletions .artifacts/adr-1-use-screen-behaviour-for-github-prs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# ADR: Use Screen Behaviour Pattern for GitHub PRs Screen

## Status

Proposed

## Context

The ticket requests implementing a GitHub Assigned PRs screen for the TRMNL playlist. The ticket description suggests creating a new Phoenix controller (`TrmnlWeb.GithubPrsController`) and a separate template file. However, the existing codebase already has a well-defined pattern for screens:

- Modules implement the `Trmnl.Screen` behaviour with a `render/1` callback
- `Trmnl.Playlist` maintains a list of screen modules
- `TrmnlWeb.ScreenController` delegates rendering to the current screen module via `ScreenHTML`
- The existing `Trmnl.Screens.HelloWorld` demonstrates this pattern using `Phoenix.Component` and HEEx

Data is fetched via the `gh` CLI tool (`System.cmd/3`) to leverage existing system authentication, avoiding manual GitHub token management.

## Decision

1. **Follow the existing screen module pattern** rather than creating a new controller/template. The new screen will be `Trmnl.Screens.GithubPrs`, implementing the `Trmnl.Screen` behaviour with an inline HEEx template via `Phoenix.Component`.

2. **Use `System.cmd/3` with the `gh` CLI** for data fetching, as specified in the ticket. The function will be extracted as a public `fetch_prs/0` for testability.

3. **Implement a simple `time_ago/1` function** using `DateTime.diff/2` rather than adding a dependency like Timex, since the requirement is straightforward (seconds/minutes/hours/days/weeks/months ago).

4. **Make the CLI runner injectable** by accepting an optional function parameter, enabling unit tests to mock the CLI output without actually calling `gh`.

## Consequences

### Positive
- Consistent with existing codebase patterns — no new routing or controller plumbing needed
- Simple, self-contained module that is easy to test and maintain
- No new dependencies required
- CLI-based auth avoids token management complexity

### Negative
- Depends on `gh` CLI being installed and authenticated on the host system
- `System.cmd/3` is a blocking call during render — acceptable for the TRMNL use case where screens are pre-rendered to BMP
- The `time_ago/1` function is custom code that must be maintained, though it is simple enough to not warrant a library dependency
6 changes: 5 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,8 @@ npm-debug.log
*.db
*.db-*

.DS_Store
.DS_Store
# UdinCode planning artifacts
.artifacts/*
!.artifacts/adr-*.md
.claude/
5 changes: 4 additions & 1 deletion lib/trmnl/inventory/device.ex
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ defmodule Trmnl.Inventory.Device do

defp upcase(changeset, fields) do
Enum.reduce(fields, changeset, fn field, changeset ->
update_change(changeset, field, &String.upcase/1)
update_change(changeset, field, fn
nil -> nil
val -> String.upcase(val)
end)
end)
end
end
3 changes: 2 additions & 1 deletion lib/trmnl/playlist.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ defmodule Trmnl.Playlist do
def screens(_device) do
# The device argument could be used to customize the playlist
[
Trmnl.Screens.HelloWorld
Trmnl.Screens.HelloWorld,
Trmnl.Screens.GithubPrs
]
end

Expand Down
87 changes: 49 additions & 38 deletions lib/trmnl/screen.ex
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,18 @@ defmodule Trmnl.Screen do
def regenerate(device) do
# --- Render the screen ---
Logger.debug("Generating screen for device #{device.id}...")
screen_path = render_current_bmp(device)

# --- Update the device ---
Inventory.update_device(device, %{
latest_screen: screen_path,
screen_generated_at: DateTime.utc_now()
})
case render_current_bmp(device) do
{:ok, screen_path} ->
Inventory.update_device(device, %{
latest_screen: screen_path,
screen_generated_at: DateTime.utc_now()
})

{:error, reason} ->
Logger.warning("Failed to generate screen for device #{device.id}: #{reason}")
{:error, reason}
end
end

# Hits the /screens/:api_key endpoint to get the current screen, then converts it to a BMP
Expand All @@ -46,37 +51,43 @@ defmodule Trmnl.Screen do

File.mkdir_p!(static_dir)

# --- Generate screenshot ---
System.cmd("puppeteer-img", [
"--width",
to_string(@width),
"--height",
to_string(@height),
"--path",
screenshot_path,
current_screen_url
])

# --- Convert screenshot to BMP ---
System.cmd("magick", [
screenshot_path,
"-resize",
"#{@width}x#{@height}",
"-dither",
"FloydSteinberg",
"-remap",
"pattern:gray50",
"-depth",
"#{@color_depth}",
"-strip",
"-monochrome",
"bmp3:#{static_path}"
])

# --- Remove temporary files ---
File.rm!(screenshot_path)

# --- Return the path to the generated BMP ---
"/generated/#{basename}.bmp"
try do
# --- Generate screenshot ---
System.cmd("puppeteer-img", [
"--width",
to_string(@width),
"--height",
to_string(@height),
"--path",
screenshot_path,
current_screen_url
])

# --- Convert screenshot to BMP ---
System.cmd("magick", [
screenshot_path,
"-resize",
"#{@width}x#{@height}",
"-dither",
"FloydSteinberg",
"-remap",
"pattern:gray50",
"-depth",
"#{@color_depth}",
"-strip",
"-monochrome",
"bmp3:#{static_path}"
])

# --- Remove temporary files ---
File.rm!(screenshot_path)

# --- Return the path to the generated BMP ---
{:ok, "/generated/#{basename}.bmp"}
rescue
e ->
File.rm(screenshot_path)
{:error, Exception.message(e)}
end
end
end
122 changes: 122 additions & 0 deletions lib/trmnl/screens/github_prs.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
defmodule Trmnl.Screens.GithubPrs do
@behaviour Trmnl.Screen

use Phoenix.Component

@assignee "bars0udin"
@fetch_timeout_ms 10_000

def render(assigns) do
prs = fetch_prs()

assigns =
assigns
|> Map.put(:count, length(prs))
|> Map.put(
:prs,
Enum.map(prs, fn pr ->
ago =
case DateTime.from_iso8601(pr["createdAt"] || "") do
{:ok, created_at, _offset} -> time_ago(created_at)
{:error, _} -> "unknown"
end

Map.put(pr, "ago", ago)
end)
)

~H"""
<div class="w-full h-full p-6 flex flex-col">
<div class="flex items-center justify-between mb-4">
<h1 class="text-2xl font-bold">GitHub PRs</h1>
<span class="text-xl font-semibold border-2 border-black rounded-full px-3 py-1">
{@count}
</span>
</div>

<%= if @count == 0 do %>
<div class="flex-1 flex items-center justify-center text-xl">
No open PRs assigned
</div>
<% else %>
<div class="flex-1 overflow-hidden">
<div class="space-y-2">
<%= for pr <- @prs do %>
<div class="flex items-baseline justify-between border-b border-black pb-2">
<span class="text-base font-medium truncate mr-4">
#{pr["number"]} {pr["title"]}
</span>
<span class="text-sm whitespace-nowrap">
{pr["ago"]}
</span>
</div>
<% end %>
</div>
</div>
<% end %>
</div>
"""
end

def fetch_prs(runner \\ &default_runner/2) do
case runner.("gh", [
"pr",
"list",
"--assignee=#{@assignee}",
"--state=open",
"--json",
"number,title,url,createdAt"
]) do
{json, 0} -> parse_prs(json)
_ -> []
end
end

defp default_runner(cmd, args) do
task = Task.async(fn -> System.cmd(cmd, args, stderr_to_stdout: true) end)

case Task.yield(task, @fetch_timeout_ms) || Task.shutdown(task) do
{:ok, result} -> result
nil -> {"timeout", 1}
end
end

def parse_prs(json) do
case Jason.decode(json) do
{:ok, prs} when is_list(prs) -> prs
_ -> []
end
end

def time_ago(datetime) do
diff = DateTime.diff(DateTime.utc_now(), datetime, :second)

cond do
diff < 0 ->
"just now"

diff < 60 ->
"#{diff} seconds ago"

diff < 3600 ->
minutes = div(diff, 60)
if minutes == 1, do: "1 minute ago", else: "#{minutes} minutes ago"

diff < 86400 ->
hours = div(diff, 3600)
if hours == 1, do: "1 hour ago", else: "#{hours} hours ago"

diff < 86400 * 7 ->
days = div(diff, 86400)
if days == 1, do: "1 day ago", else: "#{days} days ago"

diff < 86400 * 30 ->
weeks = div(diff, 86400 * 7)
if weeks == 1, do: "1 week ago", else: "#{weeks} weeks ago"

true ->
months = div(diff, 86400 * 30)
if months == 1, do: "1 month ago", else: "#{months} months ago"
end
end
end
3 changes: 1 addition & 2 deletions lib/trmnl_web/components/core_components.ex
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,7 @@ defmodule TrmnlWeb.CoreComponents do
phx-connected={hide("#client-error")}
hidden
>
Attempting to reconnect
<.icon name="hero-arrow-path" class="ml-1 h-3 w-3 animate-spin" />
Attempting to reconnect <.icon name="hero-arrow-path" class="ml-1 h-3 w-3 animate-spin" />
</.flash>

<.flash
Expand Down
2 changes: 1 addition & 1 deletion mix.lock
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"file_system": {:hex, :file_system, "1.1.0", "08d232062284546c6c34426997dd7ef6ec9f8bbd090eb91780283c9016840e8f", [:mix], [], "hexpm", "bfcf81244f416871f2a2e15c1b515287faa5db9c6bcf290222206d120b3d43f6"},
"floki": {:hex, :floki, "0.37.0", "b83e0280bbc6372f2a403b2848013650b16640cd2470aea6701f0632223d719e", [:mix], [], "hexpm", "516a0c15a69f78c47dc8e0b9b3724b29608aa6619379f91b1ffa47109b5d0dd3"},
"hackney": {:hex, :hackney, "1.20.1", "8d97aec62ddddd757d128bfd1df6c5861093419f8f7a4223823537bad5d064e2", [:rebar3], [{:certifi, "~> 2.12.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "~> 6.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "~> 1.0.0", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~> 1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.4.1", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~> 1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:unicode_util_compat, "~> 0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "fe9094e5f1a2a2c0a7d10918fee36bfec0ec2a979994cff8cfe8058cd9af38e3"},
"heroicons": {:git, "https://github.com/tailwindlabs/heroicons.git", "88ab3a0d790e6a47404cba02800a6b25d2afae50", [tag: "v2.1.1", sparse: "optimized"]},
"heroicons": {:git, "https://github.com/tailwindlabs/heroicons.git", "88ab3a0d790e6a47404cba02800a6b25d2afae50", [tag: "v2.1.1", sparse: "optimized", depth: 1]},
"hpax": {:hex, :hpax, "1.0.2", "762df951b0c399ff67cc57c3995ec3cf46d696e41f0bba17da0518d94acd4aac", [:mix], [], "hexpm", "2f09b4c1074e0abd846747329eaa26d535be0eb3d189fa69d812bfb8bfefd32f"},
"httpoison": {:hex, :httpoison, "2.2.1", "87b7ed6d95db0389f7df02779644171d7319d319178f6680438167d7b69b1f3d", [:mix], [{:hackney, "~> 1.17", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm", "51364e6d2f429d80e14fe4b5f8e39719cacd03eb3f9a9286e61e216feac2d2df"},
"idna": {:hex, :idna, "6.1.1", "8a63070e9f7d0c62eb9d9fcb360a7de382448200fbbd1b106cc96d3d8099df8d", [:rebar3], [{:unicode_util_compat, "~> 0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "92376eb7894412ed19ac475e4a86f7b413c1b9fbb5bd16dccd57934157944cea"},
Expand Down
11 changes: 10 additions & 1 deletion test/support/fixtures/inventory_fixtures.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,16 @@ defmodule Trmnl.InventoryFixtures do
@doc """
Generate a unique device mac_address.
"""
def unique_device_mac_address, do: "some mac_address#{System.unique_integer([:positive])}"
def unique_device_mac_address do
n = System.unique_integer([:positive])
# Generate a valid MAC address in AA:BB:CC:DD:EE:FF format
:io_lib.format("02:00:00:~2.16.0B:~2.16.0B:~2.16.0B", [
rem(n, 256),
rem(div(n, 256), 256),
rem(div(n, 65536), 256)
])
|> IO.iodata_to_binary()
end

@doc """
Generate a device.
Expand Down
33 changes: 26 additions & 7 deletions test/trmnl/inventory_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@ defmodule Trmnl.InventoryTest do

import Trmnl.InventoryFixtures

@invalid_attrs %{name: nil, api_key: nil, mac_address: nil, friendly_id: nil, refresh_interval: nil}
@invalid_attrs %{
name: nil,
api_key: nil,
mac_address: nil,
friendly_id: nil,
refresh_interval: nil
}

test "list_devices/0 returns all devices" do
device = device_fixture()
Expand All @@ -21,13 +27,19 @@ defmodule Trmnl.InventoryTest do
end

test "create_device/1 with valid data creates a device" do
valid_attrs = %{name: "some name", api_key: "some api_key", mac_address: "some mac_address", friendly_id: "some friendly_id", refresh_interval: 42}
valid_attrs = %{
name: "some name",
api_key: "some api_key",
mac_address: "AA:BB:CC:DD:EE:FF",
friendly_id: "some friendly_id",
refresh_interval: 42
}

assert {:ok, %Device{} = device} = Inventory.create_device(valid_attrs)
assert device.name == "some name"
assert device.api_key == "some api_key"
assert device.mac_address == "some mac_address"
assert device.friendly_id == "some friendly_id"
assert device.mac_address == "AA:BB:CC:DD:EE:FF"
assert device.friendly_id == "SOME FRIENDLY_ID"
assert device.refresh_interval == 42
end

Expand All @@ -37,13 +49,20 @@ defmodule Trmnl.InventoryTest do

test "update_device/2 with valid data updates the device" do
device = device_fixture()
update_attrs = %{name: "some updated name", api_key: "some updated api_key", mac_address: "some updated mac_address", friendly_id: "some updated friendly_id", refresh_interval: 43}

update_attrs = %{
name: "some updated name",
api_key: "some updated api_key",
mac_address: "11:22:33:44:55:66",
friendly_id: "some updated friendly_id",
refresh_interval: 43
}

assert {:ok, %Device{} = device} = Inventory.update_device(device, update_attrs)
assert device.name == "some updated name"
assert device.api_key == "some updated api_key"
assert device.mac_address == "some updated mac_address"
assert device.friendly_id == "some updated friendly_id"
assert device.mac_address == "11:22:33:44:55:66"
assert device.friendly_id == "SOME UPDATED FRIENDLY_ID"
assert device.refresh_interval == 43
end

Expand Down
Loading