From b6d79446e6da94240f4be88c22bf1e12b92bb6d5 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Fri, 13 Nov 2026 11:09:54 +1100 Subject: [PATCH 1/3] feat(place/remote_setup): set up devices from a phone Add a RemoteSetup logic driver that passes setup commands from the Concierge and Signage Manager setup flow to Booking Panel, Control and Signage devices. Implements the interface in docs/remote-setup.md of PlaceOS/user-interfaces. Co-Authored-By: Claude Opus 5.5 --- drivers/place/remote_setup.cr | 158 +++++++++++++++++++++++++++ drivers/place/remote_setup_models.cr | 88 +++++++++++++++ drivers/place/remote_setup_spec.cr | 74 +++++++++++++ 3 files changed, 320 insertions(+) create mode 100644 drivers/place/remote_setup.cr create mode 100644 drivers/place/remote_setup_models.cr create mode 100644 drivers/place/remote_setup_spec.cr diff --git a/drivers/place/remote_setup.cr b/drivers/place/remote_setup.cr new file mode 100644 index 0000000000..f4136b13fb --- /dev/null +++ b/drivers/place/remote_setup.cr @@ -0,0 +1,158 @@ +require "placeos-driver" +require "./remote_setup_models" + +# Lets a person set up a Booking Panel, Control or Signage device from a phone. +# +# Devices call `register` and wait for a command in the `device_` binding. +# The setup flow (Concierge or Signage Manager) calls `configure` or `clear`. +# The device applies the command and calls `complete`. +# +# Interface: https://github.com/PlaceOS/user-interfaces/blob/develop/docs/remote-setup.md +class Place::RemoteSetup < PlaceOS::Driver + include RemoteSetupModels + + descriptive_name "PlaceOS Remote Setup" + generic_name :RemoteSetup + description %(Passes setup commands from the Concierge and Signage Manager setup flow to Booking Panel, Control and Signage devices. + To turn it on, add `"remote_setup": {"system_id": ""}` to the authority config of the domain.) + + default_settings({ + # remove devices that did not call `register` or `complete` in this time + device_ttl_hours: 168, + # remove commands that a device did not complete in this time + command_ttl_minutes: 15, + }) + + # Most devices in the `devices` binding + MAX_DEVICES = 1000 + # Most time between a change and the `devices` binding update + PUBLISH_INTERVAL = 1.second + + @devices = {} of String => Device + # Pending command of each device, by device ID + @commands = {} of String => Command + @device_ttl : Time::Span = 7.days + @command_ttl : Time::Span = 15.minutes + # The `devices` binding is out of date and must be published + @devices_changed = false + # Only `last_seen` is out of date. Published by `remove_stale` + @last_seen_changed = false + + def on_load + self[:devices] = @devices + self[:connected] = true + on_update + + # A large fleet can register many devices at the same time, such as after + # a restart. So changes are published together at most once per interval. + schedule.every(PUBLISH_INTERVAL) { publish_devices } + schedule.every(1.minute) { remove_stale } + end + + def on_update + @device_ttl = (setting?(Int32, :device_ttl_hours) || 168).clamp(0, 24 * 365).hours + @command_ttl = (setting?(Int32, :command_ttl_minutes) || 15).clamp(0, 24 * 60).minutes + end + + # Called by a device when the driver connects, and then every 15 minutes + def register(device_id : String, app : App, config : Config? = nil) : Nil + RemoteSetupModels.check_id("device_id", device_id) + config.try &.validate! + now = Time.utc.to_unix_ms + + if device = @devices[device_id]? + if device.app == app && device.config == config + # A heartbeat. `remove_stale` publishes the new `last_seen` + @last_seen_changed = true + else + device.app = app + device.config = config + @devices_changed = true + end + device.last_seen = now + else + @devices[device_id] = Device.new(device_id, app, config, now) + @devices_changed = true + remove_oldest_devices + end + end + + # Called by the setup flow. Returns the command ID + @[Security(Level::Support)] + def configure(device_id : String, config : Config) : String + config.validate! + logger.info { "#{invoked_by_user_id} configured #{device_id} with #{config.to_json}" } + send_command(device_id, Command.new(Action::Configure, config, Time.utc.to_unix_ms)) + end + + # Called by the setup flow. Returns the command ID + @[Security(Level::Support)] + def clear(device_id : String) : String + logger.info { "#{invoked_by_user_id} cleared #{device_id}" } + send_command(device_id, Command.new(Action::Clear, nil, Time.utc.to_unix_ms)) + end + + # Called by a device after it applies a command + def complete(device_id : String, command_id : String) : Nil + command = @commands[device_id]? + return unless command && command.id == command_id + + remove_command(device_id) + if device = @devices[device_id]? + device.config = command.config + device.last_command = command.id + device.last_seen = Time.utc.to_unix_ms + @devices_changed = true + end + end + + # Remove stale devices and commands. Runs every minute + def remove_stale : Nil + now = Time.utc + device_cutoff = (now - @device_ttl).to_unix_ms + command_cutoff = (now - @command_ttl).to_unix_ms + + @commands.select { |_id, command| command.issued_at <= command_cutoff }.each_key { |id| remove_command(id) } + @devices.select { |_id, device| device.last_seen <= device_cutoff }.each_key { |id| remove_device(id) } + + @devices_changed ||= @last_seen_changed + publish_devices + end + + private def send_command(device_id : String, command : Command) : String + raise ArgumentError.new("device not found: #{device_id}") unless @devices.has_key?(device_id) + @commands[device_id] = command + self[command_binding(device_id)] = command + command.id + end + + private def remove_command(device_id : String) : Nil + # Setting `nil` deletes the binding and signals `null` + self[command_binding(device_id)] = nil if @commands.delete(device_id) + end + + private def remove_device(device_id : String) : Nil + @devices.delete(device_id) + remove_command(device_id) + @devices_changed = true + end + + # Keep `MAX_DEVICES` maximum. Devices are added one at a time, so this + # removes one device at most. + private def remove_oldest_devices : Nil + while @devices.size > MAX_DEVICES + remove_device(@devices.min_by { |_id, device| device.last_seen }[0]) + end + end + + private def publish_devices : Nil + return unless @devices_changed + @devices_changed = false + @last_seen_changed = false + self[:devices] = @devices + end + + private def command_binding(device_id : String) : String + "device_#{device_id}" + end +end diff --git a/drivers/place/remote_setup_models.cr b/drivers/place/remote_setup_models.cr new file mode 100644 index 0000000000..5ce2211ff2 --- /dev/null +++ b/drivers/place/remote_setup_models.cr @@ -0,0 +1,88 @@ +require "json" +require "random/secure" + +# Models of the RemoteSetup driver. They match the types in +# `libs/common/src/lib/remote-setup.ts` of PlaceOS/user-interfaces. +module Place::RemoteSetupModels + # Most characters in an ID that the driver accepts + MAX_ID_SIZE = 128 + + # Raise when an ID is empty or longer than `MAX_ID_SIZE` + def self.check_id(name : String, value : String) : Nil + raise ArgumentError.new("#{name} must be 1 to #{MAX_ID_SIZE} characters") unless 0 < value.size <= MAX_ID_SIZE + end + + # Application on the device. The apps use kebab-case names, such as `booking-panel` + enum App + BookingPanel + Control + Signage + + def self.new(pull : JSON::PullParser) : self + parse(pull.read_string.tr("-", "_")) + end + + def to_json(json : JSON::Builder) : Nil + json.string(to_s.underscore.tr("_", "-")) + end + end + + # Configuration that remote setup applies to a device + struct Config + include JSON::Serializable + + # System that the device shows. For signage, the display system + getter system_id : String + # Signage only. Template that the display shows + getter template_id : String? + # Booking Panel only. Open the events view of the system + getter event_view : Bool? + + def initialize(@system_id, @template_id = nil, @event_view = nil) + end + + # Raise when the IDs are not in bounds + def validate! : Nil + RemoteSetupModels.check_id("system_id", system_id) + template_id.try { |id| RemoteSetupModels.check_id("template_id", id) } + end + end + + # Device in the `devices` binding + class Device + include JSON::Serializable + + getter id : String + property app : App + # Configuration on the device. `null` when it is not set up + @[JSON::Field(emit_null: true)] + property config : Config? + # Unix epoch in milliseconds of the last `register` or `complete` call + property last_seen : Int64 + # ID of the last command that the device completed + property last_command : String? + + def initialize(@id, @app, @config, @last_seen) + end + end + + enum Action + Configure + Clear + end + + # Command in the `device_` binding. `config` is only set for `configure` + struct Command + include JSON::Serializable + + getter id : String + getter action : Action + getter config : Config? + # Unix epoch in milliseconds of the time the command was made + getter issued_at : Int64 + + def initialize(@action, @config, @issued_at) + @id = Random::Secure.hex(8) + end + end +end diff --git a/drivers/place/remote_setup_spec.cr b/drivers/place/remote_setup_spec.cr new file mode 100644 index 0000000000..4bd83ae360 --- /dev/null +++ b/drivers/place/remote_setup_spec.cr @@ -0,0 +1,74 @@ +require "placeos-driver/spec" + +DriverSpecs.mock_driver "Place::RemoteSetup" do + panel = "DEV-3F9A0C12B7E4" + command_binding = "device_#{panel}" + + it "registers devices" do + status[:connected].should eq true + status[:devices].as_h.empty?.should be_true + + exec(:register, panel, "booking-panel", nil).get + # `remove_stale` publishes the devices without waiting for the interval + exec(:remove_stale).get + + device = status[:devices][panel] + device["id"].should eq panel + device["app"].should eq "booking-panel" + device.as_h.has_key?("config").should be_true + device["config"].raw.should be_nil + end + + it "rejects bad input" do + expect_raises(PlaceOS::Driver::RemoteException) { exec(:register, "", "control", nil).get } + expect_raises(PlaceOS::Driver::RemoteException) { exec(:register, "DEV-1", "phone", nil).get } + expect_raises(PlaceOS::Driver::RemoteException) { exec(:configure, "DEV-UNKNOWN", {system_id: "sys-1"}).get } + expect_raises(PlaceOS::Driver::RemoteException) { exec(:configure, panel, {system_id: ""}).get } + end + + it "configures a device" do + command_id = exec(:configure, panel, {system_id: "sys-1", event_view: true}).get.not_nil!.as_s + + command = status[command_binding] + command["id"].should eq command_id + command["action"].should eq "configure" + command["config"]["system_id"].should eq "sys-1" + command["config"]["event_view"].should eq true + + # a different command ID does not complete the command + exec(:complete, panel, "other").get + status[command_binding]["id"].should eq command_id + + exec(:complete, panel, command_id).get + status[command_binding]?.should be_nil + + exec(:remove_stale).get + device = status[:devices][panel] + device["config"]["system_id"].should eq "sys-1" + device["last_command"].should eq command_id + end + + it "clears a device" do + command_id = exec(:clear, panel).get.not_nil!.as_s + command = status[command_binding] + command["action"].should eq "clear" + command.as_h.has_key?("config").should be_false + + exec(:complete, panel, command_id).get + exec(:remove_stale).get + device = status[:devices][panel] + device["config"].raw.should be_nil + device["last_command"].should eq command_id + end + + it "removes stale devices and commands" do + exec(:configure, panel, {system_id: "sys-1"}).get + status[command_binding]?.should_not be_nil + + settings({device_ttl_hours: 0, command_ttl_minutes: 0}) + exec(:remove_stale).get + + status[command_binding]?.should be_nil + status[:devices].as_h.empty?.should be_true + end +end From 6ebee21dd0e3ac21986fd9e6daadf11377c8cb88 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Fri, 13 Nov 2026 11:35:23 +1100 Subject: [PATCH 2/3] feat(place/remote_setup): clear by list and by system Match the updated interface in PlaceOS/user-interfaces#718: - `register` takes no config and removes the device from `clear_list` - `device_` holds configure commands only - `clear` adds the device to `clear_list`, also if it is not registered - add `clear_system` and the `clear_systems` binding - keep 1,000 entries maximum in each list Co-Authored-By: Claude Opus 5.5 --- drivers/place/remote_setup.cr | 110 +++++++++++++++++++-------- drivers/place/remote_setup_models.cr | 36 +++++---- drivers/place/remote_setup_spec.cr | 42 ++++++---- 3 files changed, 127 insertions(+), 61 deletions(-) diff --git a/drivers/place/remote_setup.cr b/drivers/place/remote_setup.cr index f4136b13fb..0d07b9cd69 100644 --- a/drivers/place/remote_setup.cr +++ b/drivers/place/remote_setup.cr @@ -3,9 +3,13 @@ require "./remote_setup_models" # Lets a person set up a Booking Panel, Control or Signage device from a phone. # -# Devices call `register` and wait for a command in the `device_` binding. -# The setup flow (Concierge or Signage Manager) calls `configure` or `clear`. -# The device applies the command and calls `complete`. +# A device on its setup screen calls `register`, then waits for a configure +# command in the `device_` binding. The setup flow (Concierge or Signage +# Manager) calls `configure`. The device applies the command and calls `complete`. +# +# A device that is set up does not call the driver. It clears its configuration +# when its ID is in `clear_list` (from `clear`), or when its app and system are +# in `clear_systems` (from `clear_system`). # # Interface: https://github.com/PlaceOS/user-interfaces/blob/develop/docs/remote-setup.md class Place::RemoteSetup < PlaceOS::Driver @@ -13,24 +17,28 @@ class Place::RemoteSetup < PlaceOS::Driver descriptive_name "PlaceOS Remote Setup" generic_name :RemoteSetup - description %(Passes setup commands from the Concierge and Signage Manager setup flow to Booking Panel, Control and Signage devices. + description %(Passes setup changes from the Concierge and Signage Manager setup flow to Booking Panel, Control and Signage devices. To turn it on, add `"remote_setup": {"system_id": ""}` to the authority config of the domain.) default_settings({ - # remove devices that did not call `register` or `complete` in this time + # remove devices that did not call `register` or `complete`, and `clear_list` IDs, after this time device_ttl_hours: 168, - # remove commands that a device did not complete in this time + # remove configure commands that a device did not complete, and `clear_systems` entries, after this time command_ttl_minutes: 15, }) - # Most devices in the `devices` binding - MAX_DEVICES = 1000 + # Most entries in each of `devices`, `clear_list` and `clear_systems` + MAX_ENTRIES = 1000 # Most time between a change and the `devices` binding update PUBLISH_INTERVAL = 1.second @devices = {} of String => Device - # Pending command of each device, by device ID + # Pending configure command of each device, by device ID @commands = {} of String => Command + # IDs of the devices to clear, with the time they were added. Oldest first + @clear_list = {} of String => Int64 + # Oldest first + @clear_systems = [] of SystemClear @device_ttl : Time::Span = 7.days @command_ttl : Time::Span = 15.minutes # The `devices` binding is out of date and must be published @@ -40,11 +48,13 @@ class Place::RemoteSetup < PlaceOS::Driver def on_load self[:devices] = @devices + publish_clear_list + publish_clear_systems self[:connected] = true on_update - # A large fleet can register many devices at the same time, such as after - # a restart. So changes are published together at most once per interval. + # Many devices can register at the same time. So `devices` changes are + # published together at most once per interval. schedule.every(PUBLISH_INTERVAL) { publish_devices } schedule.every(1.minute) { remove_stale } end @@ -54,45 +64,70 @@ class Place::RemoteSetup < PlaceOS::Driver @command_ttl = (setting?(Int32, :command_ttl_minutes) || 15).clamp(0, 24 * 60).minutes end - # Called by a device when the driver connects, and then every 15 minutes - def register(device_id : String, app : App, config : Config? = nil) : Nil + # Called by a device while it shows its setup screen, then every 15 minutes + def register(device_id : String, app : App) : Nil RemoteSetupModels.check_id("device_id", device_id) - config.try &.validate! now = Time.utc.to_unix_ms if device = @devices[device_id]? - if device.app == app && device.config == config + if device.app == app && device.config.nil? # A heartbeat. `remove_stale` publishes the new `last_seen` @last_seen_changed = true else device.app = app - device.config = config + device.config = nil @devices_changed = true end device.last_seen = now else - @devices[device_id] = Device.new(device_id, app, config, now) + @devices[device_id] = Device.new(device_id, app, now) @devices_changed = true remove_oldest_devices end + + # The device shows its setup screen, so a clear is done + publish_clear_list if @clear_list.delete(device_id) end # Called by the setup flow. Returns the command ID @[Security(Level::Support)] def configure(device_id : String, config : Config) : String config.validate! + raise ArgumentError.new("device not found: #{device_id}") unless @devices.has_key?(device_id) logger.info { "#{invoked_by_user_id} configured #{device_id} with #{config.to_json}" } - send_command(device_id, Command.new(Action::Configure, config, Time.utc.to_unix_ms)) + + command = Command.new(config, Time.utc.to_unix_ms) + @commands[device_id] = command + self[command_binding(device_id)] = command + command.id end - # Called by the setup flow. Returns the command ID + # Called by the setup flow. The device does not have to be in `devices`, + # because a device that is set up does not register. @[Security(Level::Support)] - def clear(device_id : String) : String + def clear(device_id : String) : Nil + RemoteSetupModels.check_id("device_id", device_id) logger.info { "#{invoked_by_user_id} cleared #{device_id}" } - send_command(device_id, Command.new(Action::Clear, nil, Time.utc.to_unix_ms)) + + # Delete first, so that the ID moves to the end of the list + @clear_list.delete(device_id) + @clear_list[device_id] = Time.utc.to_unix_ms + @clear_list.shift if @clear_list.size > MAX_ENTRIES + publish_clear_list + end + + # Called by the setup flow. Devices of `app` that show `system_id` clear + @[Security(Level::Support)] + def clear_system(app : App, system_id : String) : Nil + RemoteSetupModels.check_id("system_id", system_id) + logger.info { "#{invoked_by_user_id} cleared #{app.to_json} devices of #{system_id}" } + + @clear_systems << SystemClear.new(app, system_id, Time.utc.to_unix_ms) + @clear_systems.shift if @clear_systems.size > MAX_ENTRIES + publish_clear_systems end - # Called by a device after it applies a command + # Called by a device after it applies a configure command def complete(device_id : String, command_id : String) : Nil command = @commands[device_id]? return unless command && command.id == command_id @@ -106,7 +141,7 @@ class Place::RemoteSetup < PlaceOS::Driver end end - # Remove stale devices and commands. Runs every minute + # Remove stale devices, commands and clears. Runs every minute def remove_stale : Nil now = Time.utc device_cutoff = (now - @device_ttl).to_unix_ms @@ -115,17 +150,18 @@ class Place::RemoteSetup < PlaceOS::Driver @commands.select { |_id, command| command.issued_at <= command_cutoff }.each_key { |id| remove_command(id) } @devices.select { |_id, device| device.last_seen <= device_cutoff }.each_key { |id| remove_device(id) } + clear_list_size = @clear_list.size + @clear_list.reject! { |_id, added_at| added_at <= device_cutoff } + publish_clear_list if @clear_list.size != clear_list_size + + clear_systems_size = @clear_systems.size + @clear_systems.reject! { |entry| entry.issued_at <= command_cutoff } + publish_clear_systems if @clear_systems.size != clear_systems_size + @devices_changed ||= @last_seen_changed publish_devices end - private def send_command(device_id : String, command : Command) : String - raise ArgumentError.new("device not found: #{device_id}") unless @devices.has_key?(device_id) - @commands[device_id] = command - self[command_binding(device_id)] = command - command.id - end - private def remove_command(device_id : String) : Nil # Setting `nil` deletes the binding and signals `null` self[command_binding(device_id)] = nil if @commands.delete(device_id) @@ -137,10 +173,10 @@ class Place::RemoteSetup < PlaceOS::Driver @devices_changed = true end - # Keep `MAX_DEVICES` maximum. Devices are added one at a time, so this - # removes one device at most. + # Keep `MAX_ENTRIES` devices maximum. Devices are added one at a time, so + # this removes one device at most. private def remove_oldest_devices : Nil - while @devices.size > MAX_DEVICES + while @devices.size > MAX_ENTRIES remove_device(@devices.min_by { |_id, device| device.last_seen }[0]) end end @@ -152,6 +188,14 @@ class Place::RemoteSetup < PlaceOS::Driver self[:devices] = @devices end + private def publish_clear_list : Nil + self[:clear_list] = @clear_list.keys + end + + private def publish_clear_systems : Nil + self[:clear_systems] = @clear_systems + end + private def command_binding(device_id : String) : String "device_#{device_id}" end diff --git a/drivers/place/remote_setup_models.cr b/drivers/place/remote_setup_models.cr index 5ce2211ff2..d38c26e022 100644 --- a/drivers/place/remote_setup_models.cr +++ b/drivers/place/remote_setup_models.cr @@ -54,35 +54,43 @@ module Place::RemoteSetupModels getter id : String property app : App - # Configuration on the device. `null` when it is not set up + # Configuration from the last completed command. `null` after `register` @[JSON::Field(emit_null: true)] - property config : Config? + property config : Config? = nil # Unix epoch in milliseconds of the last `register` or `complete` call property last_seen : Int64 # ID of the last command that the device completed property last_command : String? - def initialize(@id, @app, @config, @last_seen) + def initialize(@id, @app, @last_seen) end end - enum Action - Configure - Clear - end - - # Command in the `device_` binding. `config` is only set for `configure` + # Configure command in the `device_` binding struct Command include JSON::Serializable - getter id : String - getter action : Action - getter config : Config? + getter id : String = Random::Secure.hex(8) + getter config : Config # Unix epoch in milliseconds of the time the command was made getter issued_at : Int64 - def initialize(@action, @config, @issued_at) - @id = Random::Secure.hex(8) + def initialize(@config, @issued_at) + end + end + + # Entry in the `clear_systems` binding. Devices of `app` that show + # `system_id` clear their configuration. + struct SystemClear + include JSON::Serializable + + getter id : String = Random::Secure.hex(8) + getter app : App + getter system_id : String + # Unix epoch in milliseconds of the time the entry was made + getter issued_at : Int64 + + def initialize(@app, @system_id, @issued_at) end end end diff --git a/drivers/place/remote_setup_spec.cr b/drivers/place/remote_setup_spec.cr index 4bd83ae360..1ad6224ce7 100644 --- a/drivers/place/remote_setup_spec.cr +++ b/drivers/place/remote_setup_spec.cr @@ -7,8 +7,10 @@ DriverSpecs.mock_driver "Place::RemoteSetup" do it "registers devices" do status[:connected].should eq true status[:devices].as_h.empty?.should be_true + status[:clear_list].as_a.empty?.should be_true + status[:clear_systems].as_a.empty?.should be_true - exec(:register, panel, "booking-panel", nil).get + exec(:register, panel, "booking-panel").get # `remove_stale` publishes the devices without waiting for the interval exec(:remove_stale).get @@ -20,10 +22,11 @@ DriverSpecs.mock_driver "Place::RemoteSetup" do end it "rejects bad input" do - expect_raises(PlaceOS::Driver::RemoteException) { exec(:register, "", "control", nil).get } - expect_raises(PlaceOS::Driver::RemoteException) { exec(:register, "DEV-1", "phone", nil).get } + expect_raises(PlaceOS::Driver::RemoteException) { exec(:register, "", "control").get } + expect_raises(PlaceOS::Driver::RemoteException) { exec(:register, "DEV-1", "phone").get } expect_raises(PlaceOS::Driver::RemoteException) { exec(:configure, "DEV-UNKNOWN", {system_id: "sys-1"}).get } expect_raises(PlaceOS::Driver::RemoteException) { exec(:configure, panel, {system_id: ""}).get } + expect_raises(PlaceOS::Driver::RemoteException) { exec(:clear_system, "signage", "").get } end it "configures a device" do @@ -31,7 +34,6 @@ DriverSpecs.mock_driver "Place::RemoteSetup" do command = status[command_binding] command["id"].should eq command_id - command["action"].should eq "configure" command["config"]["system_id"].should eq "sys-1" command["config"]["event_view"].should eq true @@ -48,20 +50,30 @@ DriverSpecs.mock_driver "Place::RemoteSetup" do device["last_command"].should eq command_id end - it "clears a device" do - command_id = exec(:clear, panel).get.not_nil!.as_s - command = status[command_binding] - command["action"].should eq "clear" - command.as_h.has_key?("config").should be_false + it "clears a device until it registers" do + # a device that is set up does not have to be in `devices` + exec(:clear, "DEV-NOT-LISTED").get + exec(:clear, panel).get + status[:clear_list].should eq ["DEV-NOT-LISTED", panel] + + # the device clears, shows its setup screen and registers + exec(:register, panel, "booking-panel").get + status[:clear_list].should eq ["DEV-NOT-LISTED"] - exec(:complete, panel, command_id).get exec(:remove_stale).get - device = status[:devices][panel] - device["config"].raw.should be_nil - device["last_command"].should eq command_id + status[:devices][panel]["config"].raw.should be_nil + end + + it "clears the devices of a system" do + exec(:clear_system, "signage", "sys-display").get + + entry = status[:clear_systems][0] + entry["id"].as_s.empty?.should be_false + entry["app"].should eq "signage" + entry["system_id"].should eq "sys-display" end - it "removes stale devices and commands" do + it "removes stale devices, commands and clears" do exec(:configure, panel, {system_id: "sys-1"}).get status[command_binding]?.should_not be_nil @@ -70,5 +82,7 @@ DriverSpecs.mock_driver "Place::RemoteSetup" do status[command_binding]?.should be_nil status[:devices].as_h.empty?.should be_true + status[:clear_list].as_a.empty?.should be_true + status[:clear_systems].as_a.empty?.should be_true end end From fb3d53f164580dc03a542118446e04a13d75b340 Mon Sep 17 00:00:00 2001 From: Alex Sorafumo Date: Fri, 13 Nov 2026 13:26:47 +1100 Subject: [PATCH 3/3] feat(place/remote_setup): configure devices that did not register Match PlaceOS/user-interfaces#718: a device that is set up also applies configure commands, so the setup flow can change its system. - `configure` does not need the device in `devices` - `complete` takes the device app and adds a device that is not listed - keep 1,000 pending commands maximum. Removing a device keeps its command Co-Authored-By: Claude Opus 5.5 --- drivers/place/remote_setup.cr | 49 +++++++++++++++++++----------- drivers/place/remote_setup_spec.cr | 18 ++++++++++- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/drivers/place/remote_setup.cr b/drivers/place/remote_setup.cr index 0d07b9cd69..db3d22657a 100644 --- a/drivers/place/remote_setup.cr +++ b/drivers/place/remote_setup.cr @@ -3,13 +3,15 @@ require "./remote_setup_models" # Lets a person set up a Booking Panel, Control or Signage device from a phone. # -# A device on its setup screen calls `register`, then waits for a configure -# command in the `device_` binding. The setup flow (Concierge or Signage -# Manager) calls `configure`. The device applies the command and calls `complete`. +# A device on its setup screen calls `register`. The setup flow (Concierge or +# Signage Manager) calls `configure`, which puts a command in the `device_` +# binding. The device applies the command and calls `complete`. A device that +# is set up also applies commands, so `configure` and `complete` work for +# devices that did not register. # -# A device that is set up does not call the driver. It clears its configuration -# when its ID is in `clear_list` (from `clear`), or when its app and system are -# in `clear_systems` (from `clear_system`). +# A device that is set up clears its configuration when its ID is in +# `clear_list` (from `clear`), or when its app and system are in +# `clear_systems` (from `clear_system`). # # Interface: https://github.com/PlaceOS/user-interfaces/blob/develop/docs/remote-setup.md class Place::RemoteSetup < PlaceOS::Driver @@ -27,13 +29,13 @@ class Place::RemoteSetup < PlaceOS::Driver command_ttl_minutes: 15, }) - # Most entries in each of `devices`, `clear_list` and `clear_systems` + # Most entries in each of `devices`, pending commands, `clear_list` and `clear_systems` MAX_ENTRIES = 1000 # Most time between a change and the `devices` binding update PUBLISH_INTERVAL = 1.second @devices = {} of String => Device - # Pending configure command of each device, by device ID + # Pending configure command of each device, by device ID. Oldest first @commands = {} of String => Command # IDs of the devices to clear, with the time they were added. Oldest first @clear_list = {} of String => Int64 @@ -89,16 +91,20 @@ class Place::RemoteSetup < PlaceOS::Driver publish_clear_list if @clear_list.delete(device_id) end - # Called by the setup flow. Returns the command ID + # Called by the setup flow. Returns the command ID. The device does not have + # to be in `devices`, because a device that is set up does not register. @[Security(Level::Support)] def configure(device_id : String, config : Config) : String + RemoteSetupModels.check_id("device_id", device_id) config.validate! - raise ArgumentError.new("device not found: #{device_id}") unless @devices.has_key?(device_id) logger.info { "#{invoked_by_user_id} configured #{device_id} with #{config.to_json}" } command = Command.new(config, Time.utc.to_unix_ms) + # Delete first, so that the command moves to the end + @commands.delete(device_id) @commands[device_id] = command self[command_binding(device_id)] = command + remove_command(@commands.first_key) if @commands.size > MAX_ENTRIES command.id end @@ -127,18 +133,25 @@ class Place::RemoteSetup < PlaceOS::Driver publish_clear_systems end - # Called by a device after it applies a configure command - def complete(device_id : String, command_id : String) : Nil + # Called by a device after it applies a configure command. `app` adds a + # device that did not register to `devices`. + def complete(device_id : String, command_id : String, app : App? = nil) : Nil command = @commands[device_id]? return unless command && command.id == command_id remove_command(device_id) - if device = @devices[device_id]? - device.config = command.config - device.last_command = command.id - device.last_seen = Time.utc.to_unix_ms - @devices_changed = true + now = Time.utc.to_unix_ms + device = @devices[device_id]? + if device.nil? && app + device = @devices[device_id] = Device.new(device_id, app, now) + remove_oldest_devices end + return unless device + + device.config = command.config + device.last_command = command.id + device.last_seen = now + @devices_changed = true end # Remove stale devices, commands and clears. Runs every minute @@ -167,9 +180,9 @@ class Place::RemoteSetup < PlaceOS::Driver self[command_binding(device_id)] = nil if @commands.delete(device_id) end + # A pending command stays, because a device that is set up can apply it private def remove_device(device_id : String) : Nil @devices.delete(device_id) - remove_command(device_id) @devices_changed = true end diff --git a/drivers/place/remote_setup_spec.cr b/drivers/place/remote_setup_spec.cr index 1ad6224ce7..4edd57194c 100644 --- a/drivers/place/remote_setup_spec.cr +++ b/drivers/place/remote_setup_spec.cr @@ -24,7 +24,7 @@ DriverSpecs.mock_driver "Place::RemoteSetup" do it "rejects bad input" do expect_raises(PlaceOS::Driver::RemoteException) { exec(:register, "", "control").get } expect_raises(PlaceOS::Driver::RemoteException) { exec(:register, "DEV-1", "phone").get } - expect_raises(PlaceOS::Driver::RemoteException) { exec(:configure, "DEV-UNKNOWN", {system_id: "sys-1"}).get } + expect_raises(PlaceOS::Driver::RemoteException) { exec(:configure, "", {system_id: "sys-1"}).get } expect_raises(PlaceOS::Driver::RemoteException) { exec(:configure, panel, {system_id: ""}).get } expect_raises(PlaceOS::Driver::RemoteException) { exec(:clear_system, "signage", "").get } end @@ -50,6 +50,22 @@ DriverSpecs.mock_driver "Place::RemoteSetup" do device["last_command"].should eq command_id end + it "configures a device that did not register" do + control = "DEV-0A1B2C3D4E5F" + command_id = exec(:configure, control, {system_id: "sys-2"}).get.not_nil!.as_s + status["device_#{control}"]["id"].should eq command_id + + # the device is set up, so it completes the command with its app + exec(:complete, control, command_id, "control").get + status["device_#{control}"]?.should be_nil + + exec(:remove_stale).get + device = status[:devices][control] + device["app"].should eq "control" + device["config"]["system_id"].should eq "sys-2" + device["last_command"].should eq command_id + end + it "clears a device until it registers" do # a device that is set up does not have to be in `devices` exec(:clear, "DEV-NOT-LISTED").get