tiny-pilot / tinypilot

Use your Raspberry Pi as a browser-based KVM.
https://tinypilotkvm.com
MIT License
3.05k stars 255 forks source link

Remove remaining video parameter defaults from `settings.yml` / bundle installer #1267

Closed jotaen4tinypilot closed 1 year ago

jotaen4tinypilot commented 1 year ago

We originally wrote all uStreamer parameters for configuring the video settings into the now-retired quick-install script (later migrated as-is to the bundle installer). The reasoning was that we wanted to selectively run parts of the uStreamer Ansible role in order to update the video settings from the UI. (See https://github.com/tiny-pilot/ansible-role-tinypilot/issues/110 and https://github.com/tiny-pilot/tinypilot/pull/657.)

This is no longer necessary, as we just abandoned the Ansible-based update mechanism of the video settings in favour of the launcher-based mechanism.

In https://github.com/tiny-pilot/tinypilot/issues/1110 / https://github.com/tiny-pilot/tinypilot/pull/1268 we already remove the defaults for tc358743/Voyager hardware.

It should be safe to clean up the remaining 3 defaults as well, i.e. for non-tc358743 hardware.

Abandoning the video parameters from settings.yml altogether means that we are slimming the surface of the settings API, so it’s worth to check whether we are able to simplify other code, e.g. in ansible-role-tinypilot.

(Note: the 2 general uStreamer parameters are maybe not eligible for clean-up, as they are still needed for server-side wiring. Per comment below, we should check this separately, though.)

Follow-up: https://github.com/tiny-pilot/tinypilot/issues/1269.

jdeanwallace commented 1 year ago

@jotaen4tinypilot

(Note: the 2 general uStreamer parameters are not eligible for clean-up, as they are still needed for server-side wiring!)

Is this correct? We already set the default ustreamer_interface and ustreamer_port in vars/main.yml which will then get saved to /opt/ustreamer-launcher/configs.d/000-defaults.yml. So does that make it safe to remove too?

jotaen4tinypilot commented 1 year ago

Good question, thanks for pointing this out. I hadn’t looked too deeply into this so far, but I updated the ticket description above to make it clear that we should check this when this ticket is picked up.