Skip to content

status ui upgrade - #61

Open
ChinmayaGit wants to merge 1 commit into
ExTV:mainfrom
ChinmayaGit:main
Open

status ui upgrade#61
ChinmayaGit wants to merge 1 commit into
ExTV:mainfrom
ChinmayaGit:main

Conversation

@ChinmayaGit

Copy link
Copy Markdown
Contributor

Add a Status dashboard that shows resource use as Azure-style metric tiles with live line charts, not bare numbers. The screen has Phone and VM image tabs. While it is open, samples refresh twice a second and a LIVE badge marks continuous updates; leaving the screen pauses sampling and clears chart history. Each tile has a title, a scrolling time-series (~1 minute), and a legend with a colored bar, metric name, resource label, and the current value.

On Phone, tiles cover CPU average, memory used percent, storage used percent, and network with separate In and Out series. On VM image, tiles cover CPU average (QEMU process load, normalized to allocated cores), emulator RSS memory, disk image size, disk activity, and VM availability (1 while running, 0 while stopped). If the VM is not running—or the backend cannot sample CPU—the chart is replaced by a short unavailable message. Detail rows under the tiles still show cores, IP, backend, allocated RAM, bandwidth, and port forwards.

@ExTV

ExTV commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Thank you for this, and sorry for the wait. There is real care in it: the lifecycle handling is right (sampling starts on resume, stops on pause, and the histories are cleared on the way out), both string files were updated together rather than leaving the Chinese translation behind, and the samplers take their clock and their reader as parameters, which is exactly how I would want them written.

I am not merging it as it stands, and the reasons below are worth going through in order. The first one is not your fault at all.

1. The branch point, which is on me

Your branch starts from main as published, and main has since moved on locally by a number of commits that are not pushed yet. Two of them rewrite the same file you did. Specifically, the Status screen now lists every address the phone holds, one row per interface, rather than a single IP. That was a bug fix: a phone on mobile data while tethering has several addresses at once, and the one it used to show was the one no PC could reach.

Merging this as it stands would quietly take that back out. GitHub reports the PR as mergeable only because it compares against the published branch, not the current one.

So the first thing this needs is a rebase once I have pushed, keeping the multi-address rows and putting the charts around them. I will comment here when the push lands so you are not rebasing onto a moving target. If you would rather I did the rebase myself, say so and I will.

2. "Network (total)" is not the phone's network

NetworkRateSampler reads TrafficStats.getUidRxBytes(Process.myUid()), which counts only Podroid's own traffic. Under a tile labelled "Network (total)" on the Phone tab, that reads as the whole device, and it is not: everything every other app does is missing from it.

This matters more than a wording nit, because it is the same shape as the bug in #53 that the multi-address change fixed: a plausible-looking number under a confident label, which is worse than showing nothing. Two ways out, either is fine: label it as Podroid's own traffic and keep it where it is, or move it to the VM tab where app-scoped traffic is the honest reading.

Worth knowing too: getUidRxBytes returns UNSUPPORTED on some devices, in which case sample() returns null forever and the tile just sits empty.

3. What the 2 Hz loop actually does

pollOnce() runs every 500 ms, and besides the cheap counters it calls:

  • NetworkUtils.localIpv4(context), which enumerates every network interface,
  • HostMetrics.snapshot(...), which hits StatFs and ActivityManager.

On the QEMU backend the phone is emulating a whole machine in software while this screen is open, so the monitor is competing for CPU with the thing it is monitoring, and an IP address does not change twice a second. Please keep the fast loop to the counters that genuinely need it (CPU deltas, traffic deltas) and move the address lookup and the storage figures to something much slower, a few seconds or on state change. The charts will look the same and the phone will notice the difference.

4. Failures that leave a blank tile

PhoneCpuSampler catches everything and returns null, and the tile then renders empty with no explanation. /proc/stat does read fine from the app on the device I tested, so this is not broken today, but it fails invisibly where it is not readable. The house rule in this repo is that a failure gets logged and surfaced rather than swallowed; the screen already has an "unavailable" message for the VM CPU case, so reusing that shape would fit.

5. Tests

There are none in the PR, and these two samplers are the easiest things in the codebase to test: you already inject the clock and the reader, so a fake pair of readings is enough to pin the delta arithmetic, the first-sample-returns-null behaviour, and the counter-reset case. app/src/test/ has the pattern to follow. MetricHistory.append deserves one too, for the cap.

Smaller things

  • The chart recomputes its peak from the data on every draw, so the y-axis moves under the line and a flat trace can look like activity. A fixed maximum for the percentage tiles would settle that.
  • MetricHistory.append builds two lists per sample per series. At ten series twice a second that is churn worth avoiding with a fixed-size buffer.
  • stateIn went from WhileSubscribed(5000) to WhileSubscribed(0). If that was deliberate, fine, but it changes behaviour on brief configuration changes.

None of this is a rejection: the direction is good and the screen looks better than what is there now. Fix 2, 3 and 4, add a couple of sampler tests, and rebase after my push, and I will take it. The current release is going out without it, since it is a set of bug fixes and this is a feature that has not been tested on hardware yet.

@ExTV

ExTV commented Jul 29, 2026

Copy link
Copy Markdown
Owner

The push has landed, so the base you were rebasing onto is no longer moving: main is now at v1.2.7 and includes the Status screen changes I mentioned.

The one to keep when you rebase is the address list. StatusUiState carries phoneAddresses, and the screen renders one row per interface under the primary address, using the phone_ip_also string. The single ui.phoneIp row your branch has would replace it, and it was a bug fix for phones holding several addresses at once, so please keep the list and put the charts around it.

StatusViewModel also changed shape in the same area, so expect the conflict to be in both files. Everything else in your PR should apply cleanly.

Along with that, the three points from my earlier review still stand: the network tile measures Podroid's own traffic rather than the phone's, the 2 Hz loop should not be re-enumerating network interfaces or hitting StatFs, and a sampler that fails should say so rather than leave a blank tile. A couple of tests on the samplers would be welcome too, since they already take their clock and reader as parameters.

If you would rather not deal with the conflict, say so and I will do the rebase myself and push it to your branch. Either way, thank you for the work; the screen is a clear improvement over what is there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants