Skip to content

Commit

Permalink
refactor: reorganize screen data API
Browse files Browse the repository at this point in the history
Key changes:

* Moves responsibility for the top-level structure of the "screen data
  API" from the `ScreenData` module into the API controller. The former
  now only has the responsibility of generating serialized widgets from
  screen configuration.

* Moves widget generation and placement from the `ScreenData` module
  into a new `Layout` module. This is a complex piece of logic with a
  small public interface, which is already shared with another module
  (`ScreenAudioData`), so it was a good candidate for extraction.

* Simplifies the interface of `ScreenData` to two functions, one for
  "normal" screen data and one for Screenplay simulation data. Pending
  config is handled by passing it in as an option. Updating the cache of
  "visible alerts" is also handled with an explicit option rather than
  implicitly whenever non-pending, non-simulation data is requested.
  • Loading branch information
digitalcora committed Sep 5, 2024
1 parent 90bd2b5 commit 8e2a976
Show file tree
Hide file tree
Showing 12 changed files with 836 additions and 878 deletions.
2 changes: 1 addition & 1 deletion config/config.exs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ config :screens,

config :screens, :screens_by_alert,
cache_module: Screens.ScreensByAlert.GenServer,
screen_data_fn: &Screens.V2.ScreenData.by_screen_id/2,
screen_data_fn: &Screens.V2.ScreenData.get/2,
screens_by_alert_ttl_seconds: 40,
screens_last_updated_ttl_seconds: 3600,
screens_ttl_seconds: 40
Expand Down
2 changes: 1 addition & 1 deletion config/test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ config :logger, level: :warning

config :screens, :screens_by_alert,
cache_module: Screens.ScreensByAlert.GenServer,
screen_data_fn: &Screens.V2.MockScreenData.by_screen_id/2,
screen_data_fn: &Screens.V2.MockScreenData.get/2,
screens_by_alert_ttl_seconds: 2,
screens_last_updated_ttl_seconds: 2,
screens_ttl_seconds: 1
9 changes: 3 additions & 6 deletions lib/screens/screens_by_alert/self_refresh_runner.ex
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ defmodule Screens.ScreensByAlert.SelfRefreshRunner do

@data_ttl_seconds 30

@screen_data_fn Application.compile_env(:screens, :screens_by_alert)[:screen_data_fn]
@screen_data_fn Application.compile_env!(:screens, [:screens_by_alert, :screen_data_fn])

@impl true
def init(:ok) do
Expand Down Expand Up @@ -68,15 +68,12 @@ defmodule Screens.ScreensByAlert.SelfRefreshRunner do
# using the return value, while also providing graceful handling of shutdowns.
#
# Doing the work in a separate, unlinked task process protects this GenServer
# process from going down if an exception is raised while running
# ScreenData.by_screen_id/1 for some screen.
# process from going down if screen data fetching raises an exception.
Enum.each(screen_ids_to_refresh, fn screen_id ->
Task.Supervisor.start_child(
TaskSupervisor,
Util.fn_with_timeout(
fn ->
@screen_data_fn.(screen_id, skip_serialize: true)
end,
fn -> @screen_data_fn.(screen_id, update_visible_alerts?: true) end,
10_000
)
)
Expand Down
12 changes: 5 additions & 7 deletions lib/screens/v2/candidate_generator.ex
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
defmodule Screens.V2.CandidateGenerator do
@moduledoc false

alias Screens.V2.ScreenData
alias Screens.V2.WidgetInstance
alias ScreensConfig.Screen

@doc """
Returns the template for this screen type.
Expand All @@ -12,16 +13,13 @@ defmodule Screens.V2.CandidateGenerator do
Fetches data and returns a list of candidate widget instances to be
considered for placement on the template.
"""
@callback candidate_instances(ScreenData.config(), keyword()) ::
ScreenData.candidate_instances()
@callback candidate_instances(Screen.t(), keyword()) :: [WidgetInstance.t()]

@doc """
Receives the finalized list of widget instances that were placed on
the template and have defined audio equivalence, as well as screen config,
and returns a list of zero or more audio-only widgets to be added to the readout.
"""
@callback audio_only_instances(
widgets :: ScreenData.candidate_instances(),
config :: ScreenData.config()
) :: ScreenData.candidate_instances()
@callback audio_only_instances(widgets :: [WidgetInstance.t()], config :: Screen.t()) ::
[WidgetInstance.t()]
end
9 changes: 5 additions & 4 deletions lib/screens/v2/screen_audio_data.ex
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
defmodule Screens.V2.ScreenAudioData do
@moduledoc false

alias Screens.Config.Cache
alias Screens.Util
alias Screens.V2.ScreenData
alias Screens.V2.ScreenData.Parameters
Expand All @@ -13,8 +14,8 @@ defmodule Screens.V2.ScreenAudioData do
@spec by_screen_id(screen_id()) :: list({module(), map()}) | :error
def by_screen_id(
screen_id,
get_config_fn \\ &ScreenData.get_config/1,
fetch_data_fn \\ &ScreenData.fetch_data/1,
get_config_fn \\ &Cache.screen/1,
generate_layout_fn \\ &ScreenData.Layout.generate/1,
get_audio_only_instances_fn \\ &get_audio_only_instances/2,
now \\ DateTime.utc_now()
) do
Expand All @@ -29,7 +30,7 @@ defmodule Screens.V2.ScreenAudioData do
if date_in_range?(audio, now) do
visual_widgets_with_audio_equivalence =
config
|> fetch_data_fn.()
|> generate_layout_fn.()
|> elem(1)
|> Map.values()
|> Enum.filter(&WidgetInstance.audio_valid_candidate?/1)
Expand All @@ -51,7 +52,7 @@ defmodule Screens.V2.ScreenAudioData do
@spec volume_by_screen_id(screen_id()) :: {:ok, float()} | :error
def volume_by_screen_id(
screen_id,
get_config_fn \\ &ScreenData.get_config/1,
get_config_fn \\ &Cache.screen/1,
now \\ DateTime.utc_now()
) do
config = get_config_fn.(screen_id)
Expand Down
Loading

0 comments on commit 8e2a976

Please sign in to comment.