Skip to content

fix: Android widget ETA and refresh failure states - #101

Open
steventeng2022 wants to merge 1 commit into
YetAnotherBusDeveloper:mainfrom
steventeng2022:codex/issue-30-widget-eta
Open

steventeng2022 wants to merge 1 commit into
YetAnotherBusDeveloper:mainfrom
steventeng2022:codex/issue-30-widget-eta

Conversation

@steventeng2022

Copy link
Copy Markdown
Contributor

Summary

  • Keep negative/missing ETA values distinct from arriving buses; ignore JSON-null messages.
  • Show request failures separately from successful empty station results, with a retry hint.
  • Preserve displayed arrivals while refreshing using a partial RemoteViews update.
  • Fetch only visible entries, deduplicate station requests, and retain the last complete-refresh timestamp on partial failure.
  • Add ETA boundary and Robolectric rendering regression tests, run by Android CI.

Related to #30. The original intermittent widget crash has not been reproduced; this PR addresses concrete display and refresh-state problems without claiming to close that report.

Validation

  • git diff --check passed.
  • Android tests added for negative/missing estimates, message precedence, arrival boundaries, station failure rendering, and hidden entries.
  • Local Android execution not completed: Flutter is unavailable on PATH. CI will run the new tests after the APK build.

Device checks still needed

  • Refresh an existing widget and confirm arrivals remain visible during loading.
  • Disable networking, refresh, then reconnect and retry.
  • Check automatic refresh under battery-saving restrictions.

@Av1anJay Av1anJay left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved — no blocking defects found in the changed widget code.

What I verified by reading the diff plus the surrounding FavoriteGroupWidgetSupport code:

  • widgetEtaText separates unavailable estimates (null/negative → --) from an actually arriving bus (0 → 進站中, 1-59 → 即將進站) and ignores blank or JSON-null messages that the previous formatEtaText rendered verbatim. The unit tests cover the boundaries, Int.MIN_VALUE, and the "null"/" NULL " message cases.
  • buildContentRemoteViews now fetches only items.take(MAX_WIDGET_ITEMS) and de-duplicates route and station requests with associateBy, so hidden entries no longer produce network work or failure state (hiddenEntriesDoNotTriggerFailures exercises exactly that, including the 6-item container count).
  • Failure surfacing is per item (更新失敗, 無法取得班次,請重試) plus a group-level 部分資料更新失敗,請點右上角重試。, and the last complete-refresh timestamp is preserved on partial failure (updateTimestamp = successfulFetches > 0 && !hasFailures).
  • The loading state uses partiallyUpdateAppWidget, so previously rendered arrivals stay visible instead of being replaced by the empty view.

Checks run: this host has no Flutter/Android toolchain (and no emulator), so I could not execute the Robolectric suites locally — I relied on CI instead. Run 37102905090 on 78c8f701 is green, and the Build Android APK job's new step "Test Android widget ETA and rendering" reports success, i.e. FavoriteWidgetRenderingTest and WidgetEtaTextTest really executed against the inflated RemoteViews (not a no-match filter). Verify (Analyze/Test/Web geometry) also passes.

Non-blocking suggestion: the new step filters to tw.avianjay.taiwanbus.flutter.*Widget*Test, so the other Kotlin test classes under android/app/src/test (LastBusMessageTest, RouteTripMonitorServiceTest, TripNotificationTextTest, YABusApplicationTest) still never run in CI — no other workflow invokes :app:testDebugUnitTest. Running the task without --tests, or with tw.avianjay.taiwanbus.flutter.*Test, would give those existing suites coverage too. See the inline note.

Comment thread .github/workflows/build.yml

This branch has not been deployed

No deployments
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.

2 participants