Replace isort with ruff. ruff is *way* faster and does the same checks.
It gives us about a 35x performance improvement over isort:
### isort
```bash
$ time isort \
--recursive \
--force-single-line-imports \
--top log \
--diff \
--check-only \
--skip-glob=third_party/* \
--skip-glob=venv/*
Skipped 2073 files
real 0m1.975s
user 0m1.743s
sys 0m0.229s
```
### ruff
```bash
$ time ruff check \
app/ \
scripts/render-template \
scripts/update-service
real 0m0.063s
user 0m0.014s
sys 0m0.073s
```
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1632"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
[ruff](https://docs.astral.sh/ruff/) applies the same checks as
pyflakes, but it has additional customizations and supports a wider
range of rules, so let's replace pyflakes with the equivalent ruff
checks.
Replacing pyflakes isn't so interesting because pyflakes is already
pretty fast, but we'll get a nice speedup from replacing isort (see
#1632) and consolidating dev tools.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1631"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
In #776, we added the `--allow-releaseinfo-change-suite` to avoid
erroring out if the Raspbian apt repo changed suite names.
In #1604, we deleted the `apt update` command entirely, as it seemed
unnecessary after deleting Ansible.
In #1623, we brought back the `apt update` command, but we left off the
flag we were previously using.
In recent installs, I'm seeing this warning:
```bash
4 packages can be upgraded. Run 'apt list --upgradable' to see them.
N: Repository 'http://raspbian.raspberrypi.org/raspbian bullseye InRelease' changed its 'Suite' value from 'stable' to 'oldstable'
Reading package lists... Done
```
For the sake of preserving longstanging behavior, I think we should
bring back the `--allow-releaseinfo-change-suite` flag.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1630"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Related https://github.com/tiny-pilot/tinypilot/issues/1026
This PR adds an API endpoint that accepts text, parses each text
character to a HID keystroke, and asynchronously sends all keystrokes to
the target machine.
### Notes
1. By writing the keystrokes in a background thread, separate from main
thread that runs the HTTP server and SocketIO server, we avoid blocking
HTTP requests and WebSocket messages ([that unexpectedly disconnected
the client during a large
paste](https://github.com/tiny-pilot/tinypilot/issues/1026)).
* The background thread that writes the pasted text, is run in a "fire
and forget" manner which means we no longer report on the success or
failure of the HID file IO. However, a single write error will abort
further keystrokes from being written.
### Peer testing
You can test this PR on device via the following stacked PR:
* https://github.com/tiny-pilot/tinypilot/pull/1626
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1625"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Resolves https://github.com/tiny-pilot/tinypilot/issues/1578
This PR overrides the default Janus systemd config in order set
additional systemd settings. Specifically:
```diff
[Unit]
Description=Janus WebRTC gateway
After=network.target
Documentation=https://janus.conf.meetecho.com/docs/index.html
+# Give up if we restart on failure 20 times within 5 minutes (300 seconds).
+StartLimitIntervalSec=300
+StartLimitBurst=20
[Service]
Type=forking
ExecStart=/usr/bin/janus --disable-colors --daemon --log-stdout
Restart=on-failure
+RestartSec=1
LimitNOFILE=65536
[Install]
WantedBy=multi-user.target
```
Janus with STUN requires the network to be online, otherwise `janus`
fails to start. Theses additional settings are needed to allow Janus
with STUN settings a better chance of starting up successfully, by
giving it more chances to restart on failure. Seeing as STUN is an
optional setting, we didn't want Janus to always require an online
network because it might unnecessarily delay TinyPilot's H.264 video
stream.
### Testing
To enable STUN, append the following block to `/etc/janus/janus.jcfg`:
```
nat: {
stun_server = "stun.gmx.de"
stun_port = 3478
}
```
Now if you restart your device, the `janus` service should start
successfully on boot. On my device `janus` restarted 8 times in 12
seconds before successfully running.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1622"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Resolves https://github.com/tiny-pilot/tinypilot/issues/1487
This PR refactors our remote-screen WebRTC API to allow media tracks to
be added/removed without automatically enabling/disabling the WebRTC
streaming mode. This gives more responsibility to the (function) caller,
but also gives more control in deciding when to change streaming modes.
Demo video:
https://github.com/tiny-pilot/tinypilot/assets/6730025/d891fbcb-1cea-41fe-b775-948b5a9879e6
Notes
1. We've renamed `enableWebrtcStreamTrack` -> `addWebrtcStreamTrack`
* This method no longer enables WebRTC mode (i.e., shows the video
element)
1. We've renamed `disableWebrtcStreamTrack` -> `removeWebrtcStreamTrack`
* This method no longer disables WebRTC mode (i.e., hides the video
element)
1. WebRTC mode can now both be enabled and have zero media tracks
* This was the cause of the original bug
1. The remote-screen will still failover to MJPEG when the connection to
Janus fails, but will not try to reconnect until a full page refresh
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1619"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Resolves#1598Resolves#1596
This reimplements the `load-tc358743-edid` service without Ansible,
which completes our process of eliminating Ansible from TinyPilot's
install process.
The last real piece was moving the installation of the
`load-tc358743-edid` from Ansible to Debian. The rest is just deleting
remaining Ansible files and stray references to Ansible.
Note that in deleting the Ansible environment setup, we delete `apt-get
update --allow-releaseinfo-change-suite` which we added in
https://github.com/tiny-pilot/tinypilot/issues/764. I don't think we
need this command anymore, but we can bring it back in the future if we
find that we need it.
## Manual tests
I ran the following manual tests:
* Verified keyboard and mouse still worked
* Verified that I could still adjust video settings
* Verified that audio still worked in H.264 (on Voyager systems)
For the following scenarios:
- [x] Install on a bare Raspbian system
- [x] Install on a bare Raspbian system with TC358743 enabled
- [x] Install a Pro Voyager image
- [x] Install a Pro Hobbyist image
- [x] Install a bundle on a Pro 2.6.0 Voyager image to simulate an
update
## Peer testing
To test this bundle run:
```bash
curl \
--silent \
--show-error \
--location \
https://raw.githubusercontent.com/tiny-pilot/tinypilot/master/scripts/install-bundle | \
sudo bash -s -- \
https://output.circle-artifacts.com/output/job/a5f0e73a-fc7b-4025-b6ce-c8f05c4be707/artifacts/0/bundler/dist/tinypilot-community-20230901T1513Z-1.9.0-109+51dc0af.tgz && \
sudo reboot
```
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1604"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
---------
Co-authored-by: David Brown <dave@sterki.co>
Related https://github.com/tiny-pilot/tinypilot-pro/issues/1030.
Raspberry Pi OS now updates the kernel on 32-bit installations to the
64-bit kernel while simultaneously keeping the userland as 32-bit, which
means that it isn't possible to accurately determine if the user is
running a 64-bit system based on the output of `uname --all`.
This change adds additional information to the `collect-debug-logs`
script, allowing us to detect when a user is running TinyPilot on an
unsupported architecture. It also slightly reformats the relevant
section for readability.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1611"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Our e2e tests make sure that links work for a project that has a license
hosted outside the device (at a web URL). Our current test uses
cryptography as this project, but we're removing cryptography as a
dependency in #1596 and #1604, so we need to pick another project to
exercise in our e2e tests.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1614"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Resolves#1595
As part of our continuing effort to eliminate Ansible from our install
process, this installs yq from within the bundle installer rather than
during Ansible.
This is a slight implementation change in that we no longer download the
yq binary at install time and instead pre-download it and include it in
the TinyPilot bundle. This is preferable, as it does more work in the
packaging stage and less work on user devices, though it does increase
the size of our bundles by a few megabytes.
While we're here, we're also upgrading to the latest version of yq, so
we're upgrading from `v4.30.6` to `v4.35.1`.
### Manual tests
For expediency, I tested builds that combined:
* #1605
* #1606
* #1608
For the manual tests, I verified:
* Video works under MJPEG and H.264
* Changing video settings works
In these environments:
- [x] Pro Voyager build
- [x] Community bundle on top of bare Raspbian system
- [x] Community bundle on top of bare Raspbian system with TC358743
enabled
### Peer tests
To test this bundle (which combines the stack of three PRs mentioned
above), run:
```bash
curl \
--silent \
--show-error \
--location \
https://raw.githubusercontent.com/tiny-pilot/tinypilot/master/scripts/install-bundle | \
sudo bash -s -- \
https://output.circle-artifacts.com/output/job/e70104a2-0f1f-43b7-abe9-098d6eac2799/artifacts/0/bundler/dist/tinypilot-community-20230831T0151Z-1.9.0-87+de1916a.tgz
```
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1608"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Resolves#1599
As part of our ongoing work to get rid of Ansible in our install
process, this change implements the Janus Debian package install in the
bundle install script rather than in Ansible.
This is also a functional change in that we're no longer adding the
whole debian-backports repo as an apt-get source. Instead, we're
including the specific Janus `.deb` file we want in TinyPilot's install
bundle.
### Manual tests
For expediency, I tested builds that combined:
* #1605
* #1606
* #1608
For the manual tests, I verified:
* Video works under MJPEG and H.264
* Changing video settings works
In these environments:
- [x] Pro Voyager build
- [x] Community bundle on top of bare Raspbian system
- [x] Community bundle on top of bare Raspbian system with TC358743
enabled
### Peer tests
To test this bundle (which combines the stack of three PRs mentioned
above), run:
```bash
curl \
--silent \
--show-error \
--location \
https://raw.githubusercontent.com/tiny-pilot/tinypilot/master/scripts/install-bundle | \
sudo bash -s -- \
https://output.circle-artifacts.com/output/job/e70104a2-0f1f-43b7-abe9-098d6eac2799/artifacts/0/bundler/dist/tinypilot-community-20230831T0151Z-1.9.0-87+de1916a.tgz
```
Resolves#1609
There's a bug that affects (at least) Firefox on Win10 where if the user
tries View > Dedicated Window, the window comes up blank and the JS
console has errors about "can't access dead object".
The issue seems like a bug in Firefox. What seems to be happening is
that the popup is trying to share resources with the main window, but
the browser's garbage seems to not be aware of the additional reference.
So the popup starts loading, we redirect the original window, which
causes the browser to tear down its resources, and then the popup
continues loading, but the shared resources it was trying to access have
been garbage collected in redirecting the original window.
Adding `noopener` seems to fix the issue, as it prevents the two windows
from sharing resources:
>`noopener`
>
>If this feature is set, the new window will not have access to the
originating window via Window.opener and returns null.
>
>When `noopener` is used, non-empty target names, other than _top,
_self, and _parent, are treated like _blank in terms of deciding whether
to open a new browsing context.
https://developer.mozilla.org/en-US/docs/Web/API/Window/open#parameters
Resolves#1549
Related https://github.com/tiny-pilot/tinypilot-pro/issues/972
### Overview
ustreamer-launcher is our thin wrapper around the uStreamer binary. It
reads settings from YAML files and then generates the appropriate
command-line string to execute uStreamer with command-line flags that
match the settings in the YAML files.
Previously, we installed ustreamer-launcher with Ansible. This change
performs the install through Debian packaging, as part of our ongoing
efforts to eliminate Ansible from our install.
### Design changes
The Ansible-based installer had a lot of complexity around how it
populated the base uStreamer config file at
`/opt/ustreamer-launcher/configs.d/000-defaults.yml`.
The Ansible-based installer wrote all of the non-null uStreamer settings
to `000-defaults.yml`, but this is an artifact of when
ustreamer-launcher was meant to be a general-purpose uStreamer tool
rather than something TinyPilot specific. ustreamer-launcher was
creating a `000-defaults.yml` file that was open-ended because the
client using the Ansible role could theoretically specify any Ansible
vars.
This change simplifies ustreamer-launcher to the TinyPilot use case. We
are either specifying the unique defaults for the TC358743 scenario
(e.g., Voyager) or the HDMI to USB dongle scenario (Hobbyist).
Now, instead of the installer dynamically writing arbitrary settings to
`000-defaults.yml`, that file is a symlink that can point to one of two
files.
We have two static YAML files that are always installed on the
filesystem: [one for
TC358743](f1c773c685/debian-pkg/opt/ustreamer-launcher/config-library/tc358743.yml)
and [one for HDMI to
USB](f1c773c685/debian-pkg/opt/ustreamer-launcher/config-library/hdmi-to-usb.yml).
At install time, `tinypilot.postinst` ensures that `000-defaults.yml` is
a symlink that points to either the TC358743 settings or the HDMI to USB
settings depending on whether TC358743 is enabled.
### Functional changes
This is mostly a non-functional refactoring, but for the sake of
simplicity, we're now ignoring these YAML settings:
* `ustreamer_video_path`
* `ustreamer_brightness`
* `ustreamer_tcp_nodelay`
Previously, it was possible for users to override these settings in
`/home/tinypilot/settings.yml` and ustreamer-launcher would launch
uStreamer with corresponding command-line flags. We never documented
these settings in a customer-facing place, and I don't know of any users
exercising them, so we can remove them.
We are also changing the permissions on the files in
`/opt/ustreamer-launcher`. Previously, the ownership was a mixture of
`root` and `ustreamer`. There isn't actually a need for `ustreamer` to
be an owner, so we're simplifying to `root` owning everything.
`tinypilot` still owns its override file, so the app can dynamically
make changes to settings.
### Manual tests
- Installed bundle on a clean Raspberry Pi OS Bullseye Lite system
- First install was without applying the TC358743 settings to
`/boot/config.txt`
- I verified the settings were correct, but I didn't test end-to-end
with the HDMI to USB dongle
- After that, I reinstalled after applying the settings to
`/boot/config.txt`, and the video stream worked correctly
- Installed a full Voyager microSD image on Pro
To test this bundle, run:
```bash
curl \
--silent \
--show-error \
--location \
https://raw.githubusercontent.com/tiny-pilot/tinypilot/master/scripts/install-bundle | \
sudo bash -s -- \
https://output.circle-artifacts.com/output/job/8d968bdd-9797-4437-ae4c-2754c8913f40/artifacts/0/bundler/dist/tinypilot-community-20230825T1401Z-1.9.0-80+5ff91e0.tgz
```
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1592"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Resolves#1584
We've been using Docker+QEMU to emulate 32-bit ARM on AMD64, but this PR
switches to ARM-native CircleCI instances, which skips the emulation.
This achieves a speedup on building Debian packages from about 4m down
to about 70s.
Because Debian package building is fast again, we're changing our CI
workflow to build a Debian package on every commit rather than just on
`master` or just when we manually trigger it.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1588"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
---------
Co-authored-by: Michael Lynch <git@mtlynch.io>
Part of https://github.com/tiny-pilot/tinypilot/issues/728.
The vertical resizing behaviour of the remote screen is currently so
that the screen element’s height appears to “jump” at certain steps due
to [CSS
breakpoints](069cf2d982/app/templates/custom-elements/remote-screen.html (L11-L33)),
while you resize the window vertically.
This PR implements “smooth” and step-less vertical resizing, while
maintaining a constant margin between remote screen and status-bar
during that process.
The breakpoint-less resizing is important for the “dedicated window”
(popup) that we are about to implement in
https://github.com/tiny-pilot/tinypilot/issues/728, where it would
probably feel weird if the remote screen height jumped around the way it
currently does, or if we wouldn’t be able to fill up the entire vertical
space of the popup window due to the breakpoint setup.
## Demo: After
https://github.com/tiny-pilot/tinypilot/assets/83721279/66758c7c-ce78-4f0a-8323-4990be47fe49
## Demo: Before
https://github.com/tiny-pilot/tinypilot/assets/83721279/f768b3c0-8b5b-44c3-8cbd-2ee724e46a02
## Notes
- We had introduced these breakpoints in
https://github.com/tiny-pilot/tinypilot/pull/878 originally, as kind of
“heuristic”/pragmatic approach for resizing the remote screen, without
having the remote screen visually collide with the status bar. The
breakpoints itself are arbitrary, and they don’t serve any other purpose
except for facilitating the vertical screen size while resizing the
window.
- I discovered https://github.com/tiny-pilot/tinypilot/issues/1581 while
testing this PR; so this behaviour is not a regression, but it’s
currently broken on master.
- This PR leaves a “naked” `<div>` in `index.html` which we technically
don’t need anymore. I’ll clean that up in a later PR, to avoid a noisy
whitespace diff here.
- The `page-content` CSS class is orphaned, so I’ve removed it.
I’ve tested on device with Firefox and Chrome, and I tried to check all
imaginable scaling constellations of window sizes and aspect ratios,
both with MJPEG and H.264, and both in full screen and regular window
mode. The biggest risk of this PR would be that we accidentally mess
with the mouse cursor positioning/alignment. It would be cool if you
could double-check on device as well, with a real target screen, just so
that we are safe.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1583"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Part of https://github.com/tiny-pilot/tinypilot/issues/1460.
This PR templatizes the Janus main config file, and adds a privileged
script for (re-)generating the config from the template plus from the
properties in `settings.yml` (i.e., `ustreamer_janus_stun_server` and
`ustreamer_janus_stun_port`).
## Notes
- In the Debian `postinst` routine, we deliberately delegate to a
privileged script instead of just inlining the logic ([like we do for
writing the app
settings](e305440929/debian-pkg/debian/tinypilot.postinst (L73-L80)),
for example), since we want to re-use the `configure-janus` logic in the
backend in a later step.
- Later, when calling the `configure-janus` privileged script from the
backend, we also need to restart the uStreamer and Janus systemd
services manually. To me, it felt safer and more explicit to let the
caller take care of that, instead of making the service restart part of
`configure-janus`. I also wasn’t sure we should be issuing direct calls
to `systemctl` or `/usr/sbin/service` anyways during the Debian install
procedure, instead of using `deb-systemd-invoke`.
- I debated whether we should add a comment above the `nat {}` block in
the Janus config, however, I wasn’t sure whether a generic explanation
of what STUN is would add enough value.
- Since neither `ustreamer_janus_stun_server` nor
`ustreamer_janus_stun_port` do have default values, it doesn’t make
sense to list them [in
`settings.py`](e305440929/app/update/settings.py (L24-L31)).
We currently don’t have an authoritative place anymore that lists and
documents all supported properties in `settings.yml`, so the two newly
added STUN properties are implicit and undocumented now. I have opened
https://github.com/tiny-pilot/tinypilot/issues/1573 for tracking that,
since I think this is a broader issue.
A few more things on testing:
- I have tested this PR quite a bit on device with the [latest bundle
build off
`af5ec0d`](https://output.circle-artifacts.com/output/job/29dc16fa-0b35-4b41-b920-85e61fa8883f/artifacts/0/bundler/dist/tinypilot-community-20230822T1618Z-1.9.0-66+af5ec0d.tgz),
so if you like, I’d suggest that a brief spot-check QA on your end would
suffice.
- Please note that testing this PR is only about verifying that we
generate a valid Janus config [according to the
documentation](7c7093e188/conf/janus.jcfg.sample.in (L259-L288)),
not about actually verifying the STUN mechanism itself, as the setup for
that [is quite complex and
laborious](https://github.com/tiny-pilot/tinypilotkvm.com/issues/939#issuecomment-1592775400).
We already gathered enough evidence that Janus with STUN in itself does
the job, and solves the use-case scenarios we are after.
- We [have an
issue](https://github.com/tiny-pilot/tinypilot/issues/1578) where Janus
would fail to come up right after booting, if a STUN server is
specified. We’ll fix this separately.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1579"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Resolves https://github.com/tiny-pilot/tinypilot/issues/1571.
There are several places in the app where we rely on the `~` path
shortcut, or the `$HOME` environment variable respectively. (For example
when [processing the `settings.yml`
file](https://github.com/tiny-pilot/tinypilot/blob/master/app/update/settings.py#L21).)
However, there are scenarios where the app (or parts of it) are run
under a different user, e.g. `root`, or with `sudo`, in which case `~`
would erroneously resolve to `/root` instead of `/home/tinypilot`.
This PR makes sure that the app always uses `/home/tinypilot` for
accessing TinyPilot-specific objects, independent of what the `$HOME`
environment variable contains. TinyPilot-specific objects are:
- `settings.yml`
- `logs/**` (i.e., update result logs)
- `.flask-secret-key`
- `tinypilot.db`
- `app_settings.cfg` (no change needed, as [this is already referenced
in an absolute
manner](504e7f113d/debian-pkg/debian/tinypilot.service (L12)))
I suggest that we don’t just hard-code the `/home/tinypilot` path as is,
because that would make local development more inconvenient – you always
would need to initialize and switch to a `tinypilot` user before being
able to start developing. Therefore, I introduced a new environment
variable (`TINYPILOT_HOME_DIR`), that lets us overrride the
`/home/tinypilot` default. Unfortunately, we cannot put this variable
into
[`dev_app_settings.cfg`](504e7f113d/dev_app_settings.cfg),
since the app config can only be read during a Flask request execution
context, but not at initialization time (at least not in a “clean” way).
To me, the `env.abs_path_in_home_dir` helper function (as proposed here)
wouldn’t be too bad either, though.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1572"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Resolves https://github.com/tiny-pilot/tinypilot/issues/1501Resolves#1267
This change gets rid of the `ustreamer_capture_device` Ansible var
(TC358743 vs. HDMI to USB) and the config file where we stored the
result. Instead, we'll track capture device based on whether
`dtoverlay=tc358743` appears in `/boot/config.txt`.
In theory, relying on `/boot/config.txt` will cause incorrect behavior
if a user enables the TC358743 overlay even though they're using HDMI to
USB. That's unlikely to happen in practice, so I think it's worth the
tradeoff, as this assumption eliminates a **lot** of complexity.
As a side-effect of eliminating `ustreamer_capture_device`, we can
remove a lot of cruft from the bundle install file.
### Background
We originally added the "uStreamer settings file" because we needed a
way to keep track of whether the device was using a HDMI to USB dongle
(like a DIY TinyPilot) or a TC358743-based HDMI to CSI chip (like a
Voyager).
At the time of the original implementation, the uStreamer Ansible role
was independent of TinyPilot, so it couldn't write to TinyPilot
settings. So, we created a "uStreamer settings file" at
`/home/ustreamer.config.yml` that would essentially persist the setting
for whether the system was TC358743-based so that the initiator of the
Ansible role wouldn't have to re-specify it every time.
### Implementation notes
* I'm broadening the scope a little bit by tackling #1267 at the same
time, but I feel like the major cost of both those issues is the testing
to make sure everything is correct, so I might as well fix and test both
at the same time.
* With this change, the Ansible role no longer adds the
`dtoverlay=tc358743` line to `/boot/config.txt` because it's now the
user's responsibility to do that in advance if they've got a DIY
TinyPilot with an HDMI to CSI chip. For Pro, Packer will take
responsibility for adding that line.
* The Ansible role still adds the `dtoverlay=tc358743-audio` line.
* `bundler/bundle/install` used to be responsible for creating
`/home/tinypilot/settings.yml` if it didn't already exist, but this
change moves that responsibility to `tinypilot.postinst`.
* I'm deliberately leaving a `debug` task in the Ansible role because I
think it's helpful to have the output in logs.
* This change drops the `ustreamer_persistent` var and instead always
launches ustreamer with the `--persistent` flag. The setting used to be
unofficially configurable via settings, but we're dropping that
configurability (see
https://github.com/tiny-pilot/tinypilot-pro/issues/972).
### Documentation impact
After we merge this, we need to update our
[wiki](https://github.com/tiny-pilot/tinypilot/wiki/Installation-Options#example-tc358743-hdmi-to-csi-capture-chip)
with the new TC358743 install method, which should simplify down to:
```bash
if ! grep --silent '^dtoverlay=tc358743$' /boot/config.txt; then
echo 'dtoverlay=tc358743' | sudo tee --append /boot/config.txt
fi
```
Corresponding changes in Pro:
https://github.com/tiny-pilot/tinypilot-pro/compare/no-capture-pro?expand=1
(difference is just Packer config)
### Manual tests
I performed the following manual tests to verify we're not breaking
functionality.
- [x] Verify TinyPilot Community install with HDMI to USB dongle
- [x] Verify TinyPilot Community install with HDMI to CSI bridge
- [x] Verify TinyPilot Pro Hobbyist image
- [x] Verify TinyPilot Pro Voyager image
- [x] Verify installing TinyPilot Pro bundle on top of TinyPilot Pro
2.6.0 Voyager image
- [x] Verify installing TinyPilot Pro bundle on top of TinyPilot Pro
2.6.0 Hobbyist image
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1554"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Related https://github.com/tiny-pilot/tinypilot/issues/1551
In https://github.com/tiny-pilot/tinypilot/pull/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 https://github.com/tiny-pilot/tinypilot-pro/issues/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 https://github.com/tiny-pilot/tinypilot/pull/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>
Resolves https://github.com/tiny-pilot/tinypilot/issues/1548
This PR moves the comments that explains how to trigger a code-based
bundle build from `.circleci/config.yml` to
`.circleci/continue_config.yml` and updates the contributing docs to
reference the correct CircleCI config file.
So the correct ways to [manually trigger a bundle
build](1ea663b0ee/CONTRIBUTING.md (installing-a-nightly-bundle))
is either the:
1. Code-based way
> 1. In [the CircleCI configuration](/.circleci/continue_config.yml),
change the `bundle_build_branch` parameter’s `default` value from
`master` to `<< pipeline.git.branch >>`.
> 1. Push your changes as branch to GitHub. CircleCI will now
automatically build bundles for all subsequent commits of that branch.
> 1. Before eventually merging your feature branch, remember to revert
the `bundle_build_branch` parameter to the original value (i.e.,
`master`)
3. CircleCI web UI way
> 1. On CircleCI’s overview page for the respective branch, click the
“Trigger Pipeline” button.
> 1. In the dialog, enter `bundle_build_branch` as parameter name, and
the (exact) name of the branch as parameter value.
> 1. Click the “Trigger Pipeline” button in the dialog. CircleCI will
now build a bundle for the latest commit of that branch.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1561"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Resolves#1495
Parses the output of `journalctl` into a simple “yes” or “no” result. If
the result is “yes”, the latest voltage warning is also included in the
output.
## Parsing the output of `journalctl`
I initially considered piping the output of `journalctl` into `grep` for
readability, but getting the correct exit code as well as just the
latest voltage warning was quite awkward with this approach. I therefore
settled on using just the filtering and output options that are built
into `journalctl`:
- The `-q` argument is used to suppress the unwanted `journalctl` header
text.
- The `-r` argument causes `journalctl` to output from newest to oldest.
- The `-n 1` argument causes `journalctl` to stop after outputting a
single line.
- The `-g "voltage"` argument limits output to only lines that contain
the word “voltage”.
The end result of this combination of arguments is that `journalctl`
will efficiently output only the most recent voltage warning. In
addition, the exit status of the command reflects whether or not any
matching lines were found, which allows for a simple `if <command>` test
to be used.
Short arguments were used for consistency with the majority of the rest
of the `collect-debug-logs` script.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1544"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
Resolves#1489
Parses the output of `vcgencmd get_throttled` into a simple “yes” or
“no” result.
## Decoding the output of `vcgencmd get_throttled`
We could decode the output of `vcgencmd get_throttled` to get more
details, but for our purposes we simply want to detect that nothing
relating to throttling has occurred since the device was booted, a
situation which is indicated by an output value of `0x0`.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1543"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>
