tiny-pilot / tinypilot

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

Support for specifying STUN server: backend logic (2/3) #1646

Closed jotaen4tinypilot closed 10 months ago

jotaen4tinypilot commented 11 months ago

Related https://github.com/tiny-pilot/tinypilot/issues/1460. Stacked on https://github.com/tiny-pilot/tinypilot/pull/1645.

This PR processes the STUN values in the API, and applies them to Janus.

For testing, it’s probably most convenient to use the next (and last) PR in the stack.

Notes

jotaen4tinypilot commented 11 months ago
Automated comment from CodeApprove ➜

⏳ @jdeanwallace please review this Pull Request

jotaen4tinypilot commented 10 months ago

One thing I noticed during testing, which I wanted to share here for the protocol: the configure-janus privileged script re-generate the /etc/janus/janus.jcfg Janus main config file. That file, however, is apparently also kept track of by systemd, as “source configuration” for the Janus service.

In case someone runs configure-janus (either manually, or via the UI) and then carries out a system update afterwards, systemd emits a warning about the changed source configuration to the logs. The warning itself is harmless, i.e., the update procedure just carries on. The warning would also go away after a device reboot, as systemd re-initializes the daemon anyway then.

Since we didn’t touch the service file itself, I’m not sure why systemd thinks we need to reload the daemon 🤔 We could of course consider to reload the systemd daemon after invoking configure-janus, just to make the warning message go away, but I don’t think it’s deemed necessary to do that from a technical perspective – the behaviour is either way like we’d expect it to be.

Screenshot 2023-10-10 at 16 35 05