Comments (7)
I’m not sure we can easily consolidate the instance in unset-static-ip
: the code contains additional logic to account for the --interface
flag, so it only removes marker sections that match the given network interface, and disregards any other marker section. For example, if multiple static IP configurations for different network interfaces had been added to dhcpcd.conf
, then they are each wrapped into their own markers. You can then do e.g. unset-static-ip --interface eth0
, to only remove the one particular marker section for the eth0
interface, while leaving all other marker sections untouched.
A potential solution that would come to my mind is to provide some sort of filter functionality in an otherwise generic remove-tinypilot-lines
script, to check whether a marker section is eligible for dropping. However, I’m worried that this would complicate things instead of simplifying them, partly because it might be cumbersome to express such filter clauses with bash APIs.
We originally introduced the static IP CLI scripts a year ago. I don’t know whether we actually make use of the multi-interface functionality anywhere in practice right now, and whether it’s therefore safe or not to drop it:
- We don’t need it for the static IP UI right now, though I don’t know whether we have plans for that. In the static IP UI, it’s currently not possible to specify the network interface, and it implicitly defaults to
eth0
. (I’m also actually not sure the UI can correctly deal with multiple static IP configs for different interfaces at all, or what would happen if they are present.) - We might have advertised CLI usage of the static IP scripts publicly, so some users might rely on it?
I think we could generally consider to establish an internal convention around these marker sections. My intuitive take is (was) that there should only be one marker section per file, and we must always be able to re-generate the section in its entirety. That at least would be the main benefit that I’d see in using the marker section approach over something more sophisticated. Otherwise, we have to do (conditional) parsing work to inspect the contents of the file, and that is actually what the marker section approach tries to avoid.
from tinypilot.
That being said, my question is how we’d proceed from here best? E.g.,
- We could still create a
remove-tinypilot-lines
script, but only use it inchange-hostname
and the FAQ, and not inunset-static-ip
. - We could look into refactoring the static IP logic, and trying to unify the multiple markers sections into a single one.
- We could try to find a
remove-tinypilot-lines
abstractions that somehow works for both use cases. - We do re-consider the issue altogether.
from tinypilot.
(WIP branch, including bats tests; unfortunately, I didn’t notice this problem earlier / right away.)
from tinypilot.
I think we can just drop the --interface
flag from unset-static-ip
. In other words, unset-static-ip
removes all TinyPilot-assigned static IPs from all interfaces.
We should make it error out if the user passes an --interface
flag so that we don't silently break old behavior if anyone has scripts relying on this (unlikely, but possible).
For this to break a use-case, we'd need to have a user go through a flow like this, correct?
- Assign a static IP to eth0
- Assign a static IP to wlan0
- Remove a static IP from eth0 only
I think the number of users who go through a flow like that is vanishingly small, probably zero.
I don't recall anyone ever requesting support for this. I think we just added it because "why not?" and we didn't think through the maintenance burden.
from tinypilot.
I think we can just drop the --interface flag from unset-static-ip. In other words, unset-static-ip removes all TinyPilot-assigned static IPs from all interfaces.
Okay cool, that simplifies things a lot.
For this to break a use-case, we'd need to have a user go through a flow like this, correct?
Correct. In this case two things could happen when they call the unset-static-ip
script:
- They don’t specify an
--interface
flag: the script would previously have defaulted to deleting theeth0
section. Then, it would delete all sections/interfaces. - They specify an
--interface
flag, e.g.--interface wlan0
: the script would previously have deleted only thewlan0
section. Then, it would fail with our about-to-be-added error message.
I’d do this now:
- Remove
--interface
flag fromunset-static-ip
API as discussed, to makeunset-static-ip
on par withchange-hostname
. - Introduce unified
remove-tinypilot-lines
script - Introduce bats tests
- Switch
unset-static-ip
andchange-hostname
to delegate toremove-tinypilot-lines
.
from tinypilot.
One addendum, just for the records: set-static-ip
invokes unset-static-ip
. So users will effectively only be able to configure one interface at a time, because trying to set a second one would clear any previously set configuration.
So regarding the scenario above:
- Assign a static IP to eth0
- Assign a static IP to wlan0 ← This clears the assignment from step 1, only leaving wlan0
from tinypilot.
So users will effectively only be able to configure one interface at a time, because trying to set a second one would clear any previously set configuration.
Ah, I hadn't considered that. Still, I think it's probably a rare scenario to need static IP for both WiFi and Ethernet. If we get requests for that, it shouldn't be too hard for us to revisit the feature and figure out a way to support it later.
from tinypilot.
Related Issues (20)
- Handling unidentified/death keys in the keyboard history UI
- Migrate CircleCI account mode to “GitHub OAuth app” HOT 2
- Using HTTPS causes the About dialog e2e license link tests to fail HOT 2
- Consolidate button.css into style.css
- Fix dialog initialization process HOT 7
- Unify initialization procedure of dialogs
- Investigation: Requirements for upgrading to uStreamer 6.0 HOT 7
- Use arithmetic bash comparisons for equals checks
- Touch input: improve double click emulation
- Touch input: support right click / context click
- Mark items for deprioritization (Removal from this Queue) HOT 2
- Update CONTRIBUTING docs to no longer say that we build amd64 Debian ustreamer packages
- Delete `dump-logs` dev script?
- Upgrade to Flask 3 HOT 2
- WiFi: store/manage multiple connections
- Build network status dialog for reviewing connectivity info HOT 17
- Clarify that setting a static IP only applies to Ethernet, not WiFi HOT 5
- (a) Research options to apply WiFi settings without reboot (timeboxed to 2 hours, otherwise we skip this for now and go with the reboot) HOT 1
- Failed to stream H.264 HOT 5
- Support for x86 devices? HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from tinypilot.