feat: Sectional Audio #2190
feat: Sectional Audio #2190
Conversation
| header: %Header{title: "Section Header"} | ||
| } | ||
|
|
||
| screen = struct(Screen, %{app_id: :busway_v2}) |
There was a problem hiding this comment.
Any particular reason you used struct for these? It seems equivalent to %Screen{app_id: :busway_v2}, but with less compile-time safety.
There was a problem hiding this comment.
Just to automatically apply the enforced keys that we don't care about. Creating the struct without some hardware specific fields throws a compilation error.
| @@ -54,7 +53,7 @@ defmodule Screens.V2.CandidateGenerator.Widgets.Departures do | |||
| Keyword.get(options, :departure_fetch_fn, &Departure.fetch/2), | |||
| Keyword.get(options, :post_process_fn, fn results, _config -> results end), | |||
| Keyword.get(options, :route_fetch_fn, &Route.fetch/1), | |||
| now | |||
| options[:now] | |||
There was a problem hiding this comment.
It seems like this is now effectively a required argument, rather than an "option" with a default value — if you didn't provide it, downstream code would crash when it expected a DateTime but got nil. I think if it's optional it should be a keyword with a default value, but if it's required it should be its own argument. No preference as to which.
There was a problem hiding this comment.
Yeah good point, didn't think about that. Making it optional seems fine and lines up with the rest of the list.
There was a problem hiding this comment.
Actually, I think I'm changing my mind. We already default now in the app CGs so there's already the variable available to pass to this.
cmaddox5
force-pushed
the
cm/sectional-audio
branch
from
cfb1cd6
to
e72b5af
Compare
Asana task: Create new Sectional audio
This PR adds the code necessary to enable audio on Sectionals. Luckily, the departures widget is already configured to readout so there were no widget changes necessary. The default for the config is set up so audio is always enabled. We shouldn't need to do anything after deploying.