Drop the use of the uStreamer port Ansible variable (#1563)

Related #1551

In #1562, we're moving
uStreamer from port `8001` to port `48001`. However, before we do that I
thought it might be a good time to hardcode the port number and
<s>enforce non-user-configurability</s> drop the use of the
`ustreamer_port` Ansible variable.

According to tiny-pilot/tinypilot-pro#972,
`ustreamer_port` is already considered a non-user-configurable variable.
This PR enforces `ustreamer_port` as being non-user-configurable.

This PR does the following:
* Reverts #1493
(which introduced that `_CONSTANTS` variable in
`app/update/settings.py`)
* Hardcodes the uStreamer port to `8001` in the TinyPilot NGINX config
and the uStreamer launcher script.
* Drops the use of the `ustreamer_port` Ansible variable.
* Cleans up the `/home/tinypilot/settings.yml` file by removing any
user-defined `ustreamer_port` at install/update time.

Notes
1. TinyPilot Pro would require the following additional change:
    ```diff
    # debian-pkg/debian/tinypilot.postinst
    
     KEYBOARD_PATH = '{{ tinypilot_keyboard_interface }}'
     MOUSE_PATH = '{{ tinypilot_mouse_interface }}'
    -USTREAMER_BASEURL = 'http://127.0.0.1:{{ ustreamer_port }}'
    +USTREAMER_BASEURL = 'http://127.0.0.1:8001'
    ```
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1563"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>

ansible-role-ustreamer/files/launch

@@ -19,7 +19,7 @@
 #   000-defaults.yml
 #   ---
 #   ustreamer_desired_fps: 30
-#   ustreamer_port: 8123
+#   ustreamer_resolution: 1280x720
 #
 #   100-other.yml
 #   ---
@@ -28,7 +28,7 @@
 #   Resulting uStreamer command-line:
 #   ustreamer \
 #     --desired-fps 20 \
-#     --port 8123
+#     --resolution 1280x720
 
 # Exit build script on first failure.
 set -e
@@ -85,8 +85,10 @@ append_arg_if_defined() {
   fi
 }
 
+USTREAMER_ARGS+=('--port')
+USTREAMER_ARGS+=('8001')
+
 append_arg_and_value_if_defined '.ustreamer_video_path'       '--device'
-append_arg_and_value_if_defined '.ustreamer_port'             '--port'
 append_arg_and_value_if_defined '.ustreamer_encoder'          '--encoder'
 append_arg_and_value_if_defined '.ustreamer_format'           '--format'
 append_arg_and_value_if_defined '.ustreamer_desired_fps'      '--desired-fps'

ansible-role-ustreamer/tasks/install_launcher.yml

@@ -40,7 +40,6 @@
     ustreamer_runtime_vars: "{{ ustreamer_runtime_vars | combine({ item: lookup('ansible.builtin.vars', item) }) }}"
   when: lookup('ansible.builtin.vars', item) != None
   loop:
-    - ustreamer_port
     - ustreamer_video_path
     - ustreamer_encoder
     - ustreamer_format

ansible-role-ustreamer/vars/main.yml

@@ -23,7 +23,6 @@ ustreamer_launcher_script: "{{ ustreamer_launcher_dir }}/launch"
 
 ustreamer_launcher_configs_dir: "{{ ustreamer_launcher_dir }}/configs.d"
 
-ustreamer_port: 8001 # Must match app/update/settings.py.
 # Assigning `ustreamer_h264_sink` will make the uStreamer role install the Janus
 # server on the system, and thus enable H264 video streaming.
 ustreamer_h264_sink: tinypilot::ustreamer::h264

app/update/settings.py

@@ -20,13 +20,6 @@
 
 _SETTINGS_FILE_PATH = os.path.expanduser('~/settings.yml')
 
-# Define constant values of the TinyPilot system. These constants take
-# precedence over the YAML data in _SETTINGS_FILE_PATH and _DEFAULTS. These
-# values are not user-configurable.
-_CONSTANTS = {
-    'ustreamer_port': 8001,  # Must match bundler/bundle/install.yml.
-}
-
 # Define default values for user-configurable TinyPilot settings. The YAML data
 # in _SETTINGS_FILE_PATH take precedence over these defaults.
 _DEFAULTS = {
@@ -59,12 +52,8 @@ class Settings:
     """
 
     def __init__(self, data):
-        # Merge the defaults with the data and constants. In case of conflicting
-        # keys, the precedence is:
-        # 1. Constants (highest precedence)
-        # 2. Data
-        # 3. Defaults (lowest precedence)
-        self._data = {**_DEFAULTS, **(data if data else {}), **_CONSTANTS}
+        # Merge the defaults with data, with data taking precedence.
+        self._data = {**_DEFAULTS, **(data if data else {})}
 
     def as_dict(self):
         return self._data
@@ -131,15 +120,13 @@ def _from_file(settings_file):
 
 
 def _to_file(settings, settings_file):
-    """Writes a Settings object to a file, excluding constants and defaults."""
-    # To avoid polluting the settings file with unnecessary default or constant
-    # values, we exclude them instead of hard-coding their values in the file.
-    settings_without_constants_or_defaults = {}
+    """Writes a Settings object to a file, excluding any default values."""
+    # To avoid polluting the settings file with unnecessary default values, we
+    # exclude them instead of hard-coding the defaults in the file.
+    settings_without_defaults = {}
     for key, value in settings.as_dict().items():
-        if (key in _CONSTANTS) or (key in _DEFAULTS and
-                                   value == _DEFAULTS[key]):
-            continue
-        settings_without_constants_or_defaults[key] = value
+        if (key not in _DEFAULTS) or (value != _DEFAULTS[key]):
+            settings_without_defaults[key] = value
 
-    if settings_without_constants_or_defaults:
-        yaml.safe_dump(settings_without_constants_or_defaults, settings_file)
+    if settings_without_defaults:
+        yaml.safe_dump(settings_without_defaults, settings_file)

app/update/settings_test.py

@@ -35,16 +35,11 @@ def read_mock_settings_file(self):
         with open(self.settings_file_path, encoding='utf-8') as mock_file:
             return mock_file.read()
 
-    def test_as_dict_returns_constant_and_default_settings_if_no_settings_file(
-            self):
+    def test_as_dict_returns_default_settings_if_no_settings_file_exists(self):
         settings_dict = update.settings.load().as_dict()
-        # Check constant value.
-        self.assertEqual(8001, settings_dict['ustreamer_port'])
-        # Check default value.
         self.assertEqual('/dev/hidg0',
                          settings_dict['tinypilot_keyboard_interface'])
-        # Count constant and default values.
-        self.assertEqual(7, len(settings_dict))
+        self.assertEqual(6, len(settings_dict))
 
     def test_populates_empty_file_with_blank_settings(self):
         self.make_mock_settings_file('')
@@ -78,20 +73,6 @@ def test_overwrites_value_of_existing_property(self):
 ustreamer_desired_fps: 10
 """.lstrip(), self.read_mock_settings_file())
 
-    def test_does_not_read_or_write_constant_value_to_file(self):
-        self.make_mock_settings_file("""
-ustreamer_port: 1234
-""".lstrip())
-
-        settings = update.settings.load()
-        settings_dict = settings.as_dict()
-        # Verify that the constant value of ustreamer_port took precedence over
-        # the value in the settings file.
-        self.assertEqual(8001, settings_dict['ustreamer_port'])
-        update.settings.save(settings)
-
-        self.assertEqual('', self.read_mock_settings_file())
-
     def test_does_not_populate_default_value_to_file(self):
         self.make_mock_settings_file('')
 

bundler/bundle/install

@@ -71,7 +71,6 @@ else
 fi
 
 # Set default installation settings
-yaml_set_if_undefined "${INSTALL_SETTINGS_FILE}" "ustreamer_port" "8001"
 yaml_set_if_undefined "${INSTALL_SETTINGS_FILE}" "ustreamer_persistent" "true"
 
 # Check if this system uses the TC358743 HDMI to CSI capture bridge.
@@ -99,6 +98,15 @@ if ! "${USE_TC358743_DEFAULTS}"; then
   yaml_set_if_undefined "${INSTALL_SETTINGS_FILE}" "ustreamer_resolution" "1920x1080"
 fi
 
+# Workaround to remove settings that are not user-configurable to avoid
+# inconsistent config. We can remove this on TinyPilot's next checkpoint
+# release.
+# https://github.com/tiny-pilot/tinypilot-pro/issues/978
+sed \
+  --in-place \
+  --expression '/ustreamer_port/d' \
+  "${INSTALL_SETTINGS_FILE}"
+
 echo "Final install settings:"
 cat "${INSTALL_SETTINGS_FILE}"
 

debian-pkg/usr/share/tinypilot/templates/tinypilot.conf.j2

@@ -2,7 +2,7 @@ upstream tinypilot {
     server 127.0.0.1:8000 fail_timeout=1s max_fails=600;
 }
 upstream ustreamer {
-    server 127.0.0.1:{{ ustreamer_port }} fail_timeout=1s max_fails=600;
+    server 127.0.0.1:8001 fail_timeout=1s max_fails=600;
 }
 upstream janus-ws {
     # The host and port must match the variables in