tiny-pilot / tinypilot

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

Build network status dialog for reviewing connectivity info #1814

Closed jotaen4tinypilot closed 2 months ago

jotaen4tinypilot commented 3 months ago

Originated in https://github.com/tiny-pilot/tinypilot/issues/131 (subtask (f)).

We originally wanted to add an indicator to the main UI (e.g., to the status bar), but after discussing options below, we eventually decided to add a new dialog for the user to review the network status. The information we want to display are:

As follow-up issue, we will add a button to the Static IP and WiFi dialogs that will direct the user to the Network Status dialog.


Original issue description:

We can add some indicator to the UI, so that the user can review which network interfaces are active/available on the device (Ethernet, WiFi). This can either be something in the status bar, or we add that info to the WiFi dialog directly.

In this process, we should also revisit whether and how we give feedback after the user entered their WiFi credentials in the WiFi dialog. We currently don’t verify at all whether the WiFi settings lead to a successful wireless connection – however, there are situations where this isn’t legitimately possible, or even undesired, e.g. if the user wants to preconfigure some WiFi settings without being in reach of the WiFi network.

One technical note: after enabling WiFi, it might take a while (e.g., 5–10 seconds) for the WiFi connection to become available. We also may need to account for the fact what happens if a network disconnects (e.g., if the Ethernet cable is unplugged). One potential idea could be to update the indicator periodically, to reflect these changes.

shalver-tp commented 3 months ago

Does this indicator indicate to which interfaces the TinyPilot is connected?

Or does it indicate which interface the user is using to connect to the TinyPilot at the moment?

Or both?

I think part of this will depend on the UI mockup, but there is a functionality aspect to it also.

jotaen4tinypilot commented 3 months ago

Such an indicator would indicate which interfaces are available (i.e., up and active) on the device, i.e., through which interfaces the device is currently connected to the network. For example, if the device is connected to the network via both Ethernet and WiFi, then the network indicator would report both interfaces as active.

I think we wouldn’t be able to tell what interfaces are currently active client-side, because as far as I know, the browser doesn’t provide APIs for us to determine this. For example, if the user is also connected to the network via both Ethernet and WiFi, then I’m not sure we would be able to tell which of the interfaces was being used for routing the current requests from the client to the device.

So I think we can only make statements about the device side of things. I’ve edited the initial ticket description to clarify this.

I’m happy to explore some UI options and prepare a few static UI mocks, just for us to make things more tangible, and to have a basis for further discussion.

shalver-tp commented 3 months ago

I was thinking that there was no way to functionally be able to tell which interface a user was connected to - thanks for confirming.

It doesn't 100% fit with the way "Connected" is currently employed in the bottom left, but I'm wondering if this new status could be part of the "Connected" Tool Tip that currently exists. Something to explore.

Regardless of tool tip, or a separate item, I think a mockup would be helpful at this stage. Thanks!

cghague commented 3 months ago

The status bar currently only includes information relevant to the client session (e.g., keystrokes, current video format, etc). We should keep system information separate to avoid confusion.

I suggest we follow the tried-and-tested icon approach, placing an unplugged network cable symbol and a disconnected Wi-Fi network symbol at the top right of the menu bar and toggling them to their connected variants based on active interfaces.

shalver-tp commented 3 months ago

@cghague Great idea!

I'm unsure on whether we put it top right or bottom right.

An ethernet port and a wi-fi signal.

One thing to keep in mind, we may want to expand this in the future if/when we add more ways to connect (LTE, Ethernet2, etc.)

jotaen4tinypilot commented 2 months ago

I’ve created some mockups to support our UI discussion. Please note that the mockups are not polished or pixel-perfect, but they are rather supposed to demonstrate a basic idea. If we like one approach, we can then iterate on the UI and optimize it.

Two Icons at Top Right

Two mockup variants of an indicator placed in the menu bar on the right-hand side. Those could either be coloured (red/green or white/grey) to indicate whether the respective network interface is active.

top-right

top-right-alt

Wi-Fi Icon in Status Bar

This mockup is based around the idea that the Ethernet interface is the default one, so we could display an optional WiFi indicator in the status bar that is only present when WiFi is active, and otherwise hide it or make it grey.

status-bar

Info in “Connected” Tooltip

In this mockup, the connection info is incorporated into the tooltip of the “Connected”/“Disconnected” tooltip, so the info would only be shown on demand (i.e., on hover).

tooltip

Discussion

The main question I was wondering about is how important the connectivity information is to users? Especially when we place the icons in the menu bar (top right), then I feel that it becomes very prominent in the UI through which network interface the user is currently connected. I’m unsure, however, whether the user really cares that much about this info, because maybe the primary thing that matters to them is that they are connected at all. We already give them this information, though, via the “Connected”/“Disconnected” indicator in the status bar (left-hand side).

That being said, I’m thinking it might not be too bad to incorporate the connectivity info into the tooltip of the “Connected” indicator. It’s much less prominent/noisy that way, and the user can inspect the information whenever they need. The downside is that the info is also much less discoverable in the tooltip, so it might be that users don’t find it unless we direct them to it – e.g., we could place a hint in the “success” state of the Wi-Fi dialog, which is slightly inelegant, though.

If we wanted to have the icons at top level, either in the status bar or in the menu bar, then I guess the tricky bit is to find a good icon for Ethernet. In contrast to WiFi, as far as I know, there is no universally understood icon for Ethernet. A cable/plug (first mockup) could be mistaken for power, and the computer network (second mockup) could be mistaken for general network availability.

We could also consider to only provide a WiFi icon, with the reasoning being that Ethernet is kind-of the default connection, and WiFi is more of an optional add-on. I’m not sure that really makes sense, however.

Regarding the “menu bar” vs. “status bar” question: so far, the menu bar is a purely “passive” UI element that doesn’t contain any status information, so I’m wondering whether it will be clear enough for users why some status information is shown in the status bar and some in the menu bar.

Last but not least, there is one technical implication that we need to consider: right now, we have an HTTP endpoint for checking the active network interfaces. If we want to display the information in the UI, then we need to poll this endpoint periodically, so that the information in the UI updates in a timely fashion – the more realtime we want this to feel, the higher the poll frequency must be. If we have an indicator that’s constantly visible in the UI, then we need to constantly make those requests to the device backend. I’m not sure yet whether this could cause performance problems (probably only in low bandwidth situations), but it might also spam our logs. We certainly can find a way to optimize this, but it’s also a bit of a cost<>benefit consideration.

If we only display the connectivity information on demand (e.g., in a tooltip, or in the Wi-Fi dialog directly), then we’d only have to make those polling-requests while the user is actively checking for the info.

All in all, I feel that there is not really a 100% optimal approach right now, but all of the above ideas have their ups and downs. I’d currently slightly lean towards the tooltip approach, mainly for simplicity – both from a technical and from a design point of view.

I’d be curious to hear your thoughts on this, and I’m happy to continue exploring this topic based on your feedback.

cghague commented 2 months ago

Excellent points, @jotaen4tinypilot!

This feature mainly solves the lack of feedback a user receives when pre-configuring a Wi-Fi network. As a result, it's something they should only need to see a couple of times, so we should look at less prominent options. We've also had several requests lately from users who want to see the details of their network from the web interface. With the right solution, we could fix two problems at once here.

I've been considering the possibilities, and I suggest dropping the idea of a status indicator and adding a new dialog showing the status of the TinyPilot device's network interfaces instead. The user would be redirected to the dialog whenever they change a network setting via the web interface, and they could also access it manually from a new menu item.

This dialog approach would:

I've made a (much less sophisticated) mockup of what I'm imagining:

network-status-concept

Let me know your thoughts!

shalver-tp commented 2 months ago

This is good discussion, thanks. Thanks for the mockups!

I think the Tool Tip or the Network Dialog box are good options and solve a few problems (polling, not cluttering the UI), I don't necessarily have a strong preference for either at the moment.

One thing to keep in mind is needing to potentially expand this information if/when we add more NICs in future hardware (2nd Eth, LTE).

Questions to think about: Does moving this to a network status dialog box lend itself to display more interfaces in the future in a way that the tooltip does not?

Does eventually (still no timeline) moving to Bookworm (and Network Manager) change how we would approach this?

cghague commented 2 months ago

A scrollable dialog could show as many interfaces as required and would be more touchscreen-friendly, which might be a factor given the focus of our latest release. Tooltips are good for transiently providing a small amount of information, so they aren't a good fit in this case.

Based on my understanding, the front-end web interface is essentially a display layer, and moving to Bookworm shouldn't affect our choices here. However, I'll defer to @jotaen4tinypilot on that topic, as he is more familiar with the app than I am!

jotaen4tinypilot commented 2 months ago

A scrollable dialog could show as many interfaces as required and would be more touchscreen-friendly, which might be a factor given the focus of our latest release. Tooltips are good for transiently providing a small amount of information, so they aren't a good fit in this case.

I agree, the tooltip would only be an option if all we wanted to display is the up/down state of either interface (as shown in the mockup above, something concise like ✓ Ethernet   × Wi-Fi). If we wanted to accommodate more info such as IP addresses or additional interfaces, then the tooltip wouldn’t scale well in terms of spacing.

Moving to Bookworm / Network Manager wouldn’t change any of the UI, but (if at all) only the underlying backend logic of how we retrieve the information from the system.

I’ll proceed with creating a proof-of-concept draft PR for the new network status dialog, then we can see it in action and discuss more details.

jotaen4tinypilot commented 2 months ago

I’ve started to work on a new “Network Status” dialog, which could roughly look like so:

https://github.com/user-attachments/assets/b5401227-08b6-4809-8b8e-be2ad76bb2aa

Discussion

What data to present?

What data do we want to show for each interface? Do we basically want to output everything that ifconfig would return (nicely formatted, though), or would it make sense to restrict the dialog to certain pieces of information? Especially in regards to cohesiveness with the other networking features, I’m thinking:

Semi-related to the point of which data to present would be the question whether we want to build a complete version of this new dialog right from the start, or whether it’s viable for us to approach this iteratively? For example, we could start to display the basic information that we already have, and then extend the dialog with more info once it’s released. (See also my other note below about determining the info from the system.)

Directing users from other dialogs

How do we want to direct the user to this new dialog? So far, we don’t have any UI pattern for jumping directly from one dialog to the other. One option would be to place a button at the end e.g. the WiFi dialog, so that the user can optionally (if desired) go to the “Network Status” dialog.

Screenshot 2024-07-19 at 17 29 01

We could also automatically open the “Network Status” dialog e.g. at the end of the Wi-Fi flow. I’m not sure, however, whether users really understand the connection between both dialogs then, and that the “Network Status” dialog is essentially its own dialog, independent of the WiFi dialog.

Determining the info from the system

It’s currently trivial for us to show the connection state and the WiFi SSID, because we already have backend logic/endpoints for retrieving that info. For the other bits we’d need to implement new endpoints in the backend to determine the info. From what I found out, we can use:

shalver-tp commented 2 months ago

I’ve started to work on a new “Network Status” dialog, which could roughly look like so:

I think this Network Status dialog looks nice and clean.

IP address, subnet mask, and router IP address: might be helpful when configuring static IP for the eth0 interface. Does it make sense to display this for the wlan0 interface too? MAC address: do we need this? Or what would be the use-case here? WiFi SSID: do we want to repeat this here, or is it sufficient to provide this info in the WiFi dialog? Technical interface names: shall we display the technical interface names such as eth0 and wlan0 somewhere? Or is this an implementation detail that’s unimportant to the user?

I think the data shown currently is the minimum we should show. If we wanted to show the other data, I think we would want it hidden until a user requests "more info." At the same time, I think this is a good iteration and doesn't need anything else. @cghague Do you have thoughts?

How do we want to direct the user to this new dialog? So far, we don’t have any UI pattern for jumping directly from one dialog to the other. One option would be to place a button at the end e.g. the WiFi dialog, so that the user can optionally (if desired) go to the “Network Status” dialog.

I think the dialog boxes (Network status and the Wi-Fi with the "jump" button look good).

I don't think we automatically route someone to network status after a Wi-Fi credential is input.

Semi-related to the point of which data to present would be the question whether we want to build a complete version of this new dialog right from the start, or whether it’s viable for us to approach this iteratively? For example, we could start to display the basic information that we already have, and then extend the dialog with more info once it’s released. (See also my other note below about determining the info from the system.)

I would think we iterate at this point, however, this brings up another topic. As we build out more network connectivity in the next 2-6 months, do we eventually roll a lot of this into a general purpose "Network Setup" dialog box? I would think eventually yes, otherwise we might end up with: adjust Wi-fi here, set Static IP here, setup 2nd Eth here, setup LTE here, etc.

cghague commented 2 months ago

Thanks, @jotaen4tinypilot, that's an excellent, clean design!

Data to present

My suggestion for a first iteration would be that we show the following basic information:

I'm ambivalent about showing the Wi-Fi network name in this dialog, but if we choose to include it, we should only show it when it's actively connected.

We should avoid using interface names in this dialog, at least for now, as they will confuse non-technical users. The users they'll be relevant to will already know how to find them over SSH.

I recommend against a "Show Details" button, as it's an anti-pattern that users find obstructive and confusing. Instead, we should focus on a clean design that shows only the most useful information.

Directing users from other dialogs

My preference is to offer a button to open this dialog on the success dialog shown after applying a Static IP address or storing a Wi-Fi network. An automatic redirection would make the process confusing.

Determining the info from the system

We can obtain network information in several ways, such as using the JSON output from the ip command. When you reach the implementation stage, I can help with the details if needed.

Consolidating the networking features

I mentioned this at one of our previous team meetings, so I'm naturally supportive of the idea! This dialog could be a good launchpad for network settings in the future, such as by adding buttons or links to each feature.

jotaen4tinypilot commented 2 months ago

Thanks for the feedback @shalver-tp and @cghague, it looks like we figured out a good scope for the first iteration:

(I’ve updated the initial ticket description and issue title.)

I agree that it might be better to leave out the Wi-Fi name, as it might be confusing to have the same information in multiple places.

With this, I think we solve immediate issues, and we also have a good basis for extending and iterating further.

Thanks for the tip with ip --json, I didn’t know this, and it will probably be the best way to go here.

jotaen4tinypilot commented 2 months ago

I’ve actually split out the button to its own task.

jotaen4tinypilot commented 2 months ago

I’ve opened PRs for the new dialog, you can see it in action in the last PR of the PR stack.

@cghague I used the ip -json address show eth0 command for retrieving all information. The JSON output looks like so: (irrelevant extra fields omitted)

[{
    "operstate": "UP",
    "address": "e4:5f:01:98:65:05",
    "addr_info": [{"family":"inet", "local":"192.168.12.86"}]
}]

Where I interpret:

Is that correct? (See also the backend implementation here, in case you are curious.)

cghague commented 2 months ago

Thanks, @jotaen4tinypilot!

You are correct about the output of the ip command. The inet object is for IPv4, and inet6 is for IPv6. We only need IPv4 for now. Multiple blocks with the same family will exist if/when we get to IPv6, but it should be safe to assume there will only be a single inet object until then.

You can exclude the eth0 argument to get the details for all interfaces simultaneously if that would simplify the implementation, but that's entirely up to your preference!

The demo looks excellent!