From 9f40599dd7b440b7c90e9ba8ee70fc9a1095fb15 Mon Sep 17 00:00:00 2001 From: Ibby Hadeed Date: Sat, 18 Jul 2026 19:40:51 +0400 Subject: [PATCH 01/10] Fix blank AppShell rendering and add a composition root AppShell embedded *fyne.Container directly, which only satisfies fyne.CanvasObject by method promotion; the driver's render-tree walk only recognizes a concrete *fyne.Container or a fyne.Widget, so window.SetContent(shell) painted nothing. Make AppShell a real widget (widget.BaseWidget + CreateRenderer) so this class of bug can't recur. The same anti-pattern was present in DiscoveryView, BrowserView, EditorView, and QuickstartView, which meant every view's content was invisible once nested inside AppShell's content stack even after the shell itself rendered. Convert all four to proper widgets too. Extract main()'s wiring into internal/boot.Compose, the single composition root shared by main() and the new rendered regression tests, which capture the composed window's canvas and assert on painted pixels rather than the container tree. --- gui/internal/boot/boot.go | 77 +++++++++++++++ gui/internal/boot/boot_test.go | 165 +++++++++++++++++++++++++++++++++ gui/internal/ui/browser.go | 16 +++- gui/internal/ui/discovery.go | 20 +++- gui/internal/ui/editor.go | 16 +++- gui/internal/ui/quickstart.go | 16 +++- gui/internal/ui/shell.go | 19 +++- gui/internal/ui/shell_test.go | 31 +++++++ gui/main.go | 65 +------------ 9 files changed, 354 insertions(+), 71 deletions(-) create mode 100644 gui/internal/boot/boot.go create mode 100644 gui/internal/boot/boot_test.go diff --git a/gui/internal/boot/boot.go b/gui/internal/boot/boot.go new file mode 100644 index 0000000..882d096 --- /dev/null +++ b/gui/internal/boot/boot.go @@ -0,0 +1,77 @@ +// Package boot contains the GoBAC Workstation's single composition root: +// everything that wires a fyne.App and fyne.Window into a running AppShell. +// main() and rendered-launch regression tests both call Compose so the +// exact tree that ships is the exact tree that gets tested. +package boot + +import ( + "fmt" + + "fyne.io/fyne/v2" + + "github.com/zyra/gobac/gui/internal/session" + "github.com/zyra/gobac/gui/internal/store" + "github.com/zyra/gobac/gui/internal/ui" +) + +// discoveryNavIndex, browserNavIndex, editorNavIndex, and quickstartNavIndex +// are the AppShell nav indices of the Discovery, Object Browser, Simulator +// Editor, and Quickstart views (see navLabels in internal/ui/shell.go). +const ( + discoveryNavIndex = 0 + browserNavIndex = 1 + editorNavIndex = 2 + quickstartNavIndex = 3 +) + +// Compose builds the application shell and wires it to sess: window sizing +// and main menu, persisted settings, session start (non-fatal on failure), +// close intercept, stores, every view, the Discovery-to-Browser selection +// handoff, and window.SetContent. It returns the composed shell so callers +// (and tests) can drive it further. +func Compose(a fyne.App, w fyne.Window, sess session.Session) *ui.AppShell { + w.Resize(fyne.NewSize(1100, 700)) + w.SetMainMenu(ui.NewMainMenu(a, w)) + + shell := ui.NewAppShell(a, w) + + settings := ui.LoadSettings(a) + if err := session.StartFromSettings(sess, settings.Interface, settings.Port); err != nil { + // Non-fatal: the simulator quickstart and scenario editor don't + // need a running session, so launch continues and the failure is + // surfaced in the status bar instead of aborting. + shell.SetStatus(fmt.Sprintf("Session not started: %v", err)) + } + w.SetCloseIntercept(func() { + _ = session.Shutdown(sess) + w.Close() + }) + + devices := store.NewDeviceStore() + objects := store.NewObjectCache() + + discovery := ui.NewDiscoveryView(sess, devices, shell) + shell.SetView(discoveryNavIndex, discovery) + + browser := ui.NewBrowserView(sess, objects, shell) + shell.SetView(browserNavIndex, browser) + + editor := ui.NewEditorView(shell) + shell.SetView(editorNavIndex, editor) + + quickstart := ui.NewQuickstartView(devices, shell) + shell.SetView(quickstartNavIndex, quickstart) + + if discoveryView, ok := discovery.(*ui.DiscoveryView); ok { + if browserView, ok := browser.(*ui.BrowserView); ok { + discoveryView.OnSelect = func(row store.DeviceRow) { + browserView.LoadDevice(row) + shell.Nav.Select(browserNavIndex) + } + } + } + + w.SetContent(shell) + + return shell +} diff --git a/gui/internal/boot/boot_test.go b/gui/internal/boot/boot_test.go new file mode 100644 index 0000000..36641c2 --- /dev/null +++ b/gui/internal/boot/boot_test.go @@ -0,0 +1,165 @@ +package boot + +import ( + "context" + "image" + "testing" + "time" + + "fyne.io/fyne/v2" + "fyne.io/fyne/v2/test" + + "github.com/zyra/gobac/gui/internal/session" +) + +// fakeSession is a minimal session.Session whose Start succeeds without +// touching any socket, so Compose can be exercised under the Fyne test +// driver without binding real network resources. +type fakeSession struct{} + +var _ session.Session = fakeSession{} + +func (fakeSession) Start(session.Config) error { return nil } +func (fakeSession) Stop() error { return nil } + +func (fakeSession) Discover(ctx context.Context, timeout time.Duration) (<-chan session.DeviceSummary, error) { + ch := make(chan session.DeviceSummary) + close(ch) + return ch, nil +} + +func (fakeSession) ReadProperty(ctx context.Context, dev session.Address, obj session.ObjectRef, prop uint32) ([]session.Value, error) { + return nil, nil +} + +func (fakeSession) ReadMultiple(ctx context.Context, dev session.Address, specs []session.ReadSpec) ([]session.ObjectResult, error) { + return nil, nil +} + +func (fakeSession) Write(ctx context.Context, dev session.Address, obj session.ObjectRef, w session.WriteRequest) error { + return nil +} + +// TestComposedWindowRendersNonBlank exercises the exact composition main() +// performs (via Compose) under the Fyne test driver and asserts on the +// rendered pixels, not the container tree. Before AppShell became a proper +// widget, window.SetContent(shell) drew nothing: the driver's software +// renderer didn't recognize the embedded-*fyne.Container promotion, so a +// captured canvas was a solid blank image (1-2 distinct colors). This test +// fails immediately if CreateRenderer is removed or SetContent stops being +// handed a real widget. +func TestComposedWindowRendersNonBlank(t *testing.T) { + a := test.NewApp() + w := a.NewWindow("t") + defer w.Close() + + Compose(a, w, fakeSession{}) + w.Resize(fyne.NewSize(1100, 700)) + + img := w.Canvas().Capture() + + colors := distinctColors(img) + if colors <= 50 { + t.Fatalf("captured canvas has %d distinct colors, want > 50 (a blank canvas has 1-2)", colors) + } + + // The content region (right of the nav list, above the status bar) + // must not be a single flat color either -- guards against a renderer + // that only paints the nav/status chrome while the center stack stays + // blank. + contentColors := distinctColorsInRegion(img, image.Rect(300, 0, 1100, 650)) + if contentColors <= 1 { + t.Fatalf("captured content region has %d distinct colors, want > 1", contentColors) + } +} + +// TestComposedWindowShowsNavAndStatus drives the real, composed canvas: +// selecting a different nav row must change what's painted, and the +// shell's renderer must actually contain the nav list widget somewhere in +// its object tree (not just as an internal struct field). +func TestComposedWindowShowsNavAndStatus(t *testing.T) { + a := test.NewApp() + w := a.NewWindow("t") + defer w.Close() + + shell := Compose(a, w, fakeSession{}) + w.Resize(fyne.NewSize(1100, 700)) + + shell.Nav.Select(0) + first := w.Canvas().Capture() + + shell.Nav.Select(2) + second := w.Canvas().Capture() + + if imagesEqual(first, second) { + t.Fatal("canvas capture is unchanged after selecting a different nav row") + } + + renderer := test.WidgetRenderer(shell) + if renderer == nil { + t.Fatal("test.WidgetRenderer(shell) returned a nil renderer") + } + found := false + for _, obj := range renderer.Objects() { + if objectTreeContains(obj, shell.Nav) { + found = true + break + } + } + if !found { + t.Fatal("shell's rendered object tree does not include the nav list") + } +} + +// distinctColors returns the number of distinct pixel colors in img. +func distinctColors(img image.Image) int { + return distinctColorsInRegion(img, img.Bounds()) +} + +// distinctColorsInRegion returns the number of distinct pixel colors within +// the intersection of img's bounds and region. +func distinctColorsInRegion(img image.Image, region image.Rectangle) int { + b := img.Bounds().Intersect(region) + seen := make(map[[4]uint32]struct{}) + for y := b.Min.Y; y < b.Max.Y; y++ { + for x := b.Min.X; x < b.Max.X; x++ { + r, g, bch, aCh := img.At(x, y).RGBA() + seen[[4]uint32{r, g, bch, aCh}] = struct{}{} + } + } + return len(seen) +} + +// imagesEqual reports whether a and b have identical bounds and pixels. +func imagesEqual(a, b image.Image) bool { + if a.Bounds() != b.Bounds() { + return false + } + bounds := a.Bounds() + for y := bounds.Min.Y; y < bounds.Max.Y; y++ { + for x := bounds.Min.X; x < bounds.Max.X; x++ { + ar, ag, ab, aa := a.At(x, y).RGBA() + br, bg, bb, ba := b.At(x, y).RGBA() + if ar != br || ag != bg || ab != bb || aa != ba { + return false + } + } + } + return true +} + +// objectTreeContains reports whether target is root itself or is reachable +// by recursing into root's children when root is a *fyne.Container. +func objectTreeContains(root, target fyne.CanvasObject) bool { + if root == target { + return true + } + if c, ok := root.(*fyne.Container); ok { + for _, child := range c.Objects { + if objectTreeContains(child, target) { + return true + } + } + } + return false +} diff --git a/gui/internal/ui/browser.go b/gui/internal/ui/browser.go index f2e57d9..4eb0005 100644 --- a/gui/internal/ui/browser.go +++ b/gui/internal/ui/browser.go @@ -107,8 +107,12 @@ var writeTagValues = map[string]uint8{ // BrowserView is the Object Browser navigation entry: an object tree (left) // fed by a device's Object_List, and a property table (right) fed by // Session.ReadMultiple for the selected object, with a write dialog. +// +// BrowserView is a proper widget (widget.BaseWidget + CreateRenderer) +// rather than an embedded *fyne.Container; see the identical note on +// DiscoveryView in discovery.go. type BrowserView struct { - *fyne.Container + widget.BaseWidget sess session.Session objects *store.ObjectCache @@ -143,6 +147,8 @@ type BrowserView struct { loadDone chan struct{} propsDone chan struct{} writeDone chan struct{} + + root *fyne.Container } // NewBrowserView builds the Object Browser view: an object tree bound to @@ -185,11 +191,17 @@ func NewBrowserView(sess session.Session, objects *store.ObjectCache, shell *App split := container.NewHSplit(v.tree, right) split.Offset = 0.3 - v.Container = container.NewBorder(nil, nil, nil, nil, split) + v.root = container.NewBorder(nil, nil, nil, nil, split) + v.ExtendBaseWidget(v) return v } +// CreateRenderer implements fyne.Widget. +func (v *BrowserView) CreateRenderer() fyne.WidgetRenderer { + return widget.NewSimpleRenderer(v.root) +} + // treeUID scheme: "" is the root; "g:" is a group (branch) node; // "o::" is a leaf. diff --git a/gui/internal/ui/discovery.go b/gui/internal/ui/discovery.go index 4aef694..60ad5ac 100644 --- a/gui/internal/ui/discovery.go +++ b/gui/internal/ui/discovery.go @@ -37,8 +37,16 @@ var sweepDurations = []string{"1s", "3s", "10s"} // DiscoveryView is the Discovery navigation entry: sweep controls plus a // live device table bound to a store.DeviceStore. +// +// DiscoveryView is a proper widget (widget.BaseWidget + CreateRenderer) +// rather than an embedded *fyne.Container, for the same reason AppShell is +// (see shell.go): a struct that only embeds *fyne.Container satisfies +// fyne.CanvasObject by promotion, but the driver's render-tree walk +// recognizes concrete *fyne.Container or fyne.Widget values, not types that +// merely embed one — so its children would never be painted when placed +// inside another container (e.g. AppShell.Content). type DiscoveryView struct { - *fyne.Container + widget.BaseWidget sess session.Session devices *store.DeviceStore @@ -58,6 +66,8 @@ type DiscoveryView struct { removeListener func() + root *fyne.Container + // sweepDone is a test-only synchronization seam: if non-nil when // sweep() is invoked, it is closed once that sweep's background // goroutine finishes all of its work (including re-enabling @@ -108,7 +118,8 @@ func NewDiscoveryView(sess session.Session, devices *store.DeviceStore, shell *A } } - v.Container = container.NewBorder(toolbar, nil, nil, nil, v.table) + v.root = container.NewBorder(toolbar, nil, nil, nil, v.table) + v.ExtendBaseWidget(v) v.removeListener = v.devices.AddListener(v.refresh) v.refresh() @@ -116,6 +127,11 @@ func NewDiscoveryView(sess session.Session, devices *store.DeviceStore, shell *A return v } +// CreateRenderer implements fyne.Widget. +func (v *DiscoveryView) CreateRenderer() fyne.WidgetRenderer { + return widget.NewSimpleRenderer(v.root) +} + // cellText renders the data cell at id from the cached snapshot. func (v *DiscoveryView) cellText(id widget.TableCellID) string { if id.Row < 0 || id.Row >= len(v.cached) { diff --git a/gui/internal/ui/editor.go b/gui/internal/ui/editor.go index 9a3a057..c3b2210 100644 --- a/gui/internal/ui/editor.go +++ b/gui/internal/ui/editor.go @@ -54,8 +54,12 @@ func classifyObjectType(t string) string { // EditorView is the Simulator Editor navigation entry: toolbar (New, Open, // Save, Save As), a network form, and master-detail device/object editing // over a scenariodoc.Document, with live validation. +// +// EditorView is a proper widget (widget.BaseWidget + CreateRenderer) rather +// than an embedded *fyne.Container; see the identical note on DiscoveryView +// in discovery.go. type EditorView struct { - *fyne.Container + widget.BaseWidget shell *AppShell doc *scenariodoc.Document @@ -107,6 +111,8 @@ type EditorView struct { objInitialPriorityEntry *widget.Entry objNumberOfStatesEntry *widget.Entry objCovIncrementEntry *widget.Entry + + root *fyne.Container } // NewEditorView builds the Simulator Editor view, holding a @@ -120,11 +126,17 @@ func NewEditorView(shell *AppShell) fyne.CanvasObject { selectedObject: -1, } v.buildWidgets() + v.ExtendBaseWidget(v) v.selectDevice(0) v.revalidate() return v } +// CreateRenderer implements fyne.Widget. +func (v *EditorView) CreateRenderer() fyne.WidgetRenderer { + return widget.NewSimpleRenderer(v.root) +} + // buildWidgets constructs every widget that lives for the view's whole // lifetime (the toolbar, network form, and the two list widgets). The // device/object detail forms are rebuilt per-selection by @@ -185,7 +197,7 @@ func (v *EditorView) buildWidgets() { mainSplit := container.NewHSplit(listSplit, formSplit) top := container.NewVBox(toolbar, v.titleLabel, networkForm) - v.Container = container.NewBorder(top, v.summaryLabel, nil, nil, mainSplit) + v.root = container.NewBorder(top, v.summaryLabel, nil, nil, mainSplit) } // ---- device list ---- diff --git a/gui/internal/ui/quickstart.go b/gui/internal/ui/quickstart.go index d272aba..7853c6e 100644 --- a/gui/internal/ui/quickstart.go +++ b/gui/internal/ui/quickstart.go @@ -33,8 +33,12 @@ var _ quickstartRunner = (*simrun.Runner)(nil) // in-process simulator run whose devices are injected into the shared // DeviceStore (Source "local-sim") so Discovery and the Object Browser can // exercise them over real loopback UDP. +// +// QuickstartView is a proper widget (widget.BaseWidget + CreateRenderer) +// rather than an embedded *fyne.Container; see the identical note on +// DiscoveryView in discovery.go. type QuickstartView struct { - *fyne.Container + widget.BaseWidget devices *store.DeviceStore shell *AppShell @@ -60,6 +64,8 @@ type QuickstartView struct { // Production code leaves them nil. startDone chan struct{} stopDone chan struct{} + + root *fyne.Container } // NewQuickstartView builds the Quickstart view. @@ -87,13 +93,19 @@ func NewQuickstartView(devices *store.DeviceStore, shell *AppShell) fyne.CanvasO }, ) - v.Container = container.NewBorder( + v.root = container.NewBorder( container.NewVBox(description, toolbar), nil, nil, nil, v.list, ) + v.ExtendBaseWidget(v) return v } +// CreateRenderer implements fyne.Widget. +func (v *QuickstartView) CreateRenderer() fyne.WidgetRenderer { + return widget.NewSimpleRenderer(v.root) +} + // defaultStartRunner decodes sc and starts it via simrun.Start, adapting // *simrun.Runner to the quickstartRunner interface. func defaultStartRunner(ctx context.Context, sc *simulator.Scenario) (quickstartRunner, error) { diff --git a/gui/internal/ui/shell.go b/gui/internal/ui/shell.go index d9a4dc9..63a63fb 100644 --- a/gui/internal/ui/shell.go +++ b/gui/internal/ui/shell.go @@ -18,13 +18,22 @@ var viewLabels = []string{"Discovery view", "Object browser", "Scenario editor", // AppShell is the top-level content for the GoBAC Workstation main window: // a left navigation list, a center content stack that switches per // selection, and a bottom status bar. +// +// AppShell is a proper widget (widget.BaseWidget + CreateRenderer) rather +// than an embedded *fyne.Container. Embedding *fyne.Container only +// satisfies fyne.CanvasObject by method promotion on the concrete type +// *AppShell; the driver's software renderer recognizes *fyne.Container and +// fyne.Widget by concrete type/interface, not by promotion, so a bare +// embed renders nothing when handed to window.SetContent. Being a widget +// with CreateRenderer makes that class of bug impossible here. type AppShell struct { - *fyne.Container + widget.BaseWidget Nav *widget.List Content *fyne.Container Status *widget.Label + root *fyne.Container selected int } @@ -52,11 +61,17 @@ func NewAppShell(a fyne.App, w fyne.Window) *AppShell { statusBar := container.NewHBox(shell.Status) - shell.Container = container.NewBorder(nil, statusBar, shell.Nav, nil, shell.Content) + shell.root = container.NewBorder(nil, statusBar, shell.Nav, nil, shell.Content) + shell.ExtendBaseWidget(shell) return shell } +// CreateRenderer implements fyne.Widget. +func (s *AppShell) CreateRenderer() fyne.WidgetRenderer { + return widget.NewSimpleRenderer(s.root) +} + // updateNavItem renders the navigation row at id into obj, an item created // by the List's CreateItem callback. func updateNavItem(id widget.ListItemID, obj fyne.CanvasObject) { diff --git a/gui/internal/ui/shell_test.go b/gui/internal/ui/shell_test.go index 3e1133a..76d5a6e 100644 --- a/gui/internal/ui/shell_test.go +++ b/gui/internal/ui/shell_test.go @@ -1,6 +1,7 @@ package ui import ( + "image" "testing" "time" @@ -42,8 +43,20 @@ func TestSelectingNavIndexSwitchesVisibleContent(t *testing.T) { defer w.Close() shell := NewAppShell(a, w) + w.SetContent(shell) + w.Resize(fyne.NewSize(900, 600)) + + shell.Nav.Select(0) + first := w.Canvas().Capture() shell.Nav.Select(2) + second := w.Canvas().Capture() + + // Rendered assertion: what the driver actually paints must change when + // the nav selection changes, not just internal container state. + if imagesEqual(first, second) { + t.Fatal("canvas capture is unchanged after selecting a different nav row") + } if got, want := visibleLabelText(t, shell.Content), "Scenario editor"; got != want { t.Errorf("visible content = %q, want %q", got, want) @@ -112,3 +125,21 @@ func visibleLabelText(t *testing.T, stack *fyne.Container) string { } return found } + +// imagesEqual reports whether a and b have identical bounds and pixels. +func imagesEqual(a, b image.Image) bool { + if a.Bounds() != b.Bounds() { + return false + } + bounds := a.Bounds() + for y := bounds.Min.Y; y < bounds.Max.Y; y++ { + for x := bounds.Min.X; x < bounds.Max.X; x++ { + ar, ag, ab, aa := a.At(x, y).RGBA() + br, bg, bb, ba := b.At(x, y).RGBA() + if ar != br || ag != bg || ab != bb || aa != ba { + return false + } + } + } + return true +} diff --git a/gui/main.go b/gui/main.go index 0664a7b..a16b639 100644 --- a/gui/main.go +++ b/gui/main.go @@ -2,72 +2,15 @@ package main import ( - "fmt" - - "fyne.io/fyne/v2" "fyne.io/fyne/v2/app" + "github.com/zyra/gobac/gui/internal/boot" "github.com/zyra/gobac/gui/internal/session" - "github.com/zyra/gobac/gui/internal/store" - "github.com/zyra/gobac/gui/internal/ui" -) - -// discoveryNavIndex, browserNavIndex, editorNavIndex, and quickstartNavIndex -// are the AppShell nav indices of the Discovery, Object Browser, Simulator -// Editor, and Quickstart views (see navLabels in internal/ui/shell.go). -const ( - discoveryNavIndex = 0 - browserNavIndex = 1 - editorNavIndex = 2 - quickstartNavIndex = 3 ) func main() { a := app.NewWithID("com.zyra.gobac.gui") - - window := a.NewWindow("GoBAC Workstation") - window.Resize(fyne.NewSize(1100, 700)) - window.SetMainMenu(ui.NewMainMenu(a, window)) - - shell := ui.NewAppShell(a, window) - - sess := session.NewLive() - settings := ui.LoadSettings(a) - if err := session.StartFromSettings(sess, settings.Interface, settings.Port); err != nil { - // Non-fatal: the simulator quickstart and scenario editor don't - // need a running session, so launch continues and the failure is - // surfaced in the status bar instead of aborting. - shell.SetStatus(fmt.Sprintf("Session not started: %v", err)) - } - window.SetCloseIntercept(func() { - _ = session.Shutdown(sess) - window.Close() - }) - - devices := store.NewDeviceStore() - objects := store.NewObjectCache() - - discovery := ui.NewDiscoveryView(sess, devices, shell) - shell.SetView(discoveryNavIndex, discovery) - - browser := ui.NewBrowserView(sess, objects, shell) - shell.SetView(browserNavIndex, browser) - - editor := ui.NewEditorView(shell) - shell.SetView(editorNavIndex, editor) - - quickstart := ui.NewQuickstartView(devices, shell) - shell.SetView(quickstartNavIndex, quickstart) - - if discoveryView, ok := discovery.(*ui.DiscoveryView); ok { - if browserView, ok := browser.(*ui.BrowserView); ok { - discoveryView.OnSelect = func(row store.DeviceRow) { - browserView.LoadDevice(row) - shell.Nav.Select(browserNavIndex) - } - } - } - - window.SetContent(shell) - window.ShowAndRun() + w := a.NewWindow("GoBAC Workstation") + boot.Compose(a, w, session.NewLive()) + w.ShowAndRun() } From 2254edd4a45a4975952097d2e44f36bc0b5afa9a Mon Sep 17 00:00:00 2001 From: Ibby Hadeed Date: Sat, 18 Jul 2026 19:58:25 +0400 Subject: [PATCH 02/10] Replace free-text network interface with a picked dropdown Settings now offers "Automatic (recommended)" plus each usable network interface by human-friendly label instead of a raw text field, backed by a new netpick package. Bootstrap resolves Automatic to the best real interface at start time, and Settings now restarts the live session on save instead of requiring a relaunch, reporting plain connected/failed status either way. --- gui/internal/boot/boot.go | 55 ++++++- gui/internal/boot/boot_test.go | 60 +++++++ gui/internal/netpick/netpick.go | 141 +++++++++++++++++ gui/internal/netpick/netpick_test.go | 174 +++++++++++++++++++++ gui/internal/session/bootstrap.go | 27 ++++ gui/internal/session/bootstrap_test.go | 59 +++++++ gui/internal/ui/settings.go | 75 +++++++-- gui/internal/ui/settings_test.go | 207 +++++++++++++++++++++++++ 8 files changed, 780 insertions(+), 18 deletions(-) create mode 100644 gui/internal/netpick/netpick.go create mode 100644 gui/internal/netpick/netpick_test.go diff --git a/gui/internal/boot/boot.go b/gui/internal/boot/boot.go index 882d096..d51cfaa 100644 --- a/gui/internal/boot/boot.go +++ b/gui/internal/boot/boot.go @@ -6,9 +6,11 @@ package boot import ( "fmt" + "net" "fyne.io/fyne/v2" + "github.com/zyra/gobac/gui/internal/netpick" "github.com/zyra/gobac/gui/internal/session" "github.com/zyra/gobac/gui/internal/store" "github.com/zyra/gobac/gui/internal/ui" @@ -31,17 +33,20 @@ const ( // (and tests) can drive it further. func Compose(a fyne.App, w fyne.Window, sess session.Session) *ui.AppShell { w.Resize(fyne.NewSize(1100, 700)) - w.SetMainMenu(ui.NewMainMenu(a, w)) shell := ui.NewAppShell(a, w) - settings := ui.LoadSettings(a) - if err := session.StartFromSettings(sess, settings.Interface, settings.Port); err != nil { - // Non-fatal: the simulator quickstart and scenario editor don't - // need a running session, so launch continues and the failure is - // surfaced in the status bar instead of aborting. - shell.SetStatus(fmt.Sprintf("Session not started: %v", err)) + restart := func(s ui.Settings) { + _ = session.Shutdown(sess) + startSession(sess, shell, s) } + w.SetMainMenu(ui.NewMainMenu(a, w, restart)) + + // Startup goes through the same startSession helper Settings' Restart + // callback uses, so first launch and a live network change report + // identically in the status bar. + startSession(sess, shell, ui.LoadSettings(a)) + w.SetCloseIntercept(func() { _ = session.Shutdown(sess) w.Close() @@ -75,3 +80,39 @@ func Compose(a fyne.App, w fyne.Window, sess session.Session) *ui.AppShell { return shell } + +// startSession starts sess using s and reports the outcome in shell's +// status bar with plain, human-readable wording naming the resolved +// network's label (not a raw interface name). A failure is non-fatal: the +// simulator quickstart and scenario editor don't need a running session, +// so callers keep going with just the status bar reflecting what happened. +func startSession(sess session.Session, shell *ui.AppShell, s ui.Settings) { + label := interfaceLabel(s.Interface) + if err := session.StartFromSettings(sess, s.Interface, s.Port); err != nil { + shell.SetStatus(fmt.Sprintf("Couldn't start on %s: %v", label, err)) + return + } + shell.SetStatus(fmt.Sprintf("Connected on %s (port %d)", label, s.Port)) +} + +// interfaceLabel returns the human-friendly netpick label for iface (a +// Settings.Interface value): the Automatic pick's label when iface is +// empty, or the matching candidate's label for a named interface. It falls +// back to "Automatic" or the raw name when netpick has nothing to say +// (e.g. no usable interface, or one that has since disappeared) so the +// status bar always has something plain to show. +func interfaceLabel(iface string) string { + cands := netpick.Candidates(net.Interfaces) + if iface == "" { + if c, ok := netpick.Automatic(cands); ok { + return c.Label + } + return "Automatic" + } + for _, c := range cands { + if c.Name == iface { + return c.Label + } + } + return iface +} diff --git a/gui/internal/boot/boot_test.go b/gui/internal/boot/boot_test.go index 36641c2..c73a195 100644 --- a/gui/internal/boot/boot_test.go +++ b/gui/internal/boot/boot_test.go @@ -2,7 +2,9 @@ package boot import ( "context" + "errors" "image" + "strings" "testing" "time" @@ -10,6 +12,7 @@ import ( "fyne.io/fyne/v2/test" "github.com/zyra/gobac/gui/internal/session" + "github.com/zyra/gobac/gui/internal/ui" ) // fakeSession is a minimal session.Session whose Start succeeds without @@ -40,6 +43,63 @@ func (fakeSession) Write(ctx context.Context, dev session.Address, obj session.O return nil } +// failingSession is a session.Session whose Start always fails, so +// Compose's failure-path status wording can be exercised without touching +// any socket. +type failingSession struct{ fakeSession } + +func (failingSession) Start(session.Config) error { return errors.New("bind failed") } + +// awaitStatus polls shell's rendered status label until it is non-empty or +// the deadline passes, then returns its text. SetStatus dispatches via +// fyne.Do, so a freshly-composed shell's status may not be set yet on the +// calling goroutine. +func awaitStatus(t *testing.T, shell *ui.AppShell) string { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if s := shell.Status.Text; s != "" { + return s + } + time.Sleep(time.Millisecond) + } + t.Fatal("status bar text was never set") + return "" +} + +// TestComposeReportsConnectedStatusOnSuccessfulStart exercises the plain +// success wording Compose's startup path (and, by the same helper, a +// Settings restart) reports in the rendered status bar. +func TestComposeReportsConnectedStatusOnSuccessfulStart(t *testing.T) { + a := test.NewApp() + w := a.NewWindow("t") + defer w.Close() + + shell := Compose(a, w, fakeSession{}) + + got := awaitStatus(t, shell) + if !strings.HasPrefix(got, "Connected on ") || !strings.Contains(got, "(port ") { + t.Errorf("status = %q, want prefix %q and a port", got, "Connected on ") + } +} + +// TestComposeReportsFailureStatusWhenStartFails exercises the plain +// failure wording when the session fails to start; launch must still +// continue (Compose returns a usable shell) since other views don't depend +// on a running session. +func TestComposeReportsFailureStatusWhenStartFails(t *testing.T) { + a := test.NewApp() + w := a.NewWindow("t") + defer w.Close() + + shell := Compose(a, w, failingSession{}) + + got := awaitStatus(t, shell) + if !strings.HasPrefix(got, "Couldn't start on ") || !strings.Contains(got, "bind failed") { + t.Errorf("status = %q, want prefix %q containing %q", got, "Couldn't start on ", "bind failed") + } +} + // TestComposedWindowRendersNonBlank exercises the exact composition main() // performs (via Compose) under the Fyne test driver and asserts on the // rendered pixels, not the container tree. Before AppShell became a proper diff --git a/gui/internal/netpick/netpick.go b/gui/internal/netpick/netpick.go new file mode 100644 index 0000000..0e4f4df --- /dev/null +++ b/gui/internal/netpick/netpick.go @@ -0,0 +1,141 @@ +// Package netpick turns the OS network interface list into a small set of +// human-friendly candidates for the settings UI: which interfaces are usable +// (up, with an IPv4 address), what to call them, and which one to pick +// automatically when the user hasn't chosen one. It has no dependency on +// Fyne so it stays unit-testable on its own, matching the rest of this +// module's non-ui packages. +package netpick + +import ( + "fmt" + "net" + "sort" + "strings" +) + +// Candidate is one selectable network interface. +type Candidate struct { + // Name is the OS interface name (e.g. "eno0", "lo"). This is what gets + // persisted to settings -- never the Label, which embeds a volatile IP. + Name string + // Label is the human-friendly display string, e.g. + // "Wired (eno0) — 192.168.1.5". + Label string + // IPv4 is the interface's first usable IPv4 address, in dotted form. + IPv4 string + // Loopback reports whether this is a loopback interface (used by the + // simulator, so it's listed rather than filtered out). + Loopback bool +} + +// Candidates lists the usable network interfaces: those reported up by +// list (production passes net.Interfaces) that have at least one IPv4 +// address. list is a seam so tests can supply a fake interface set without +// touching real hardware. Results are sorted non-loopback first, then by +// name. +func Candidates(list func() ([]net.Interface, error)) []Candidate { + ifaces, err := list() + if err != nil { + return nil + } + + var out []Candidate + for _, ifc := range ifaces { + if ifc.Flags&net.FlagUp == 0 { + continue + } + addrs, err := ifc.Addrs() + if err != nil { + continue + } + ipv4, ok := firstIPv4(addrs) + if !ok { + continue + } + loopback := ifc.Flags&net.FlagLoopback != 0 + out = append(out, Candidate{ + Name: ifc.Name, + Label: label(ifc.Name, ipv4, loopback), + IPv4: ipv4, + Loopback: loopback, + }) + } + + sortCandidates(out) + + return out +} + +// sortCandidates orders cands non-loopback first, then by name, in place. +func sortCandidates(cands []Candidate) { + sort.SliceStable(cands, func(i, j int) bool { + if cands[i].Loopback != cands[j].Loopback { + return !cands[i].Loopback + } + return cands[i].Name < cands[j].Name + }) +} + +// Automatic picks the candidate the GUI should use when the user has not +// chosen one explicitly: the first non-loopback candidate, falling back to +// the first loopback candidate, and reporting false when cands is empty. +func Automatic(cands []Candidate) (Candidate, bool) { + for _, c := range cands { + if !c.Loopback { + return c, true + } + } + for _, c := range cands { + if c.Loopback { + return c, true + } + } + return Candidate{}, false +} + +// firstIPv4 returns the first IPv4 address among addrs, in dotted form. +func firstIPv4(addrs []net.Addr) (string, bool) { + for _, addr := range addrs { + var ip net.IP + switch v := addr.(type) { + case *net.IPNet: + ip = v.IP + case *net.IPAddr: + ip = v.IP + default: + continue + } + if ip4 := ip.To4(); ip4 != nil { + return ip4.String(), true + } + } + return "", false +} + +// friendlyName maps an interface name to a human-friendly kind, by prefix: +// "wl*" (wireless) -> "Wi-Fi", "en*"/"eth*" (wired) -> "Wired", "lo*" +// (loopback) -> "Loopback (testing)", anything else -> "Network". +func friendlyName(name string) string { + switch { + case strings.HasPrefix(name, "wl"): + return "Wi-Fi" + case strings.HasPrefix(name, "en"), strings.HasPrefix(name, "eth"): + return "Wired" + case strings.HasPrefix(name, "lo"): + return "Loopback (testing)" + default: + return "Network" + } +} + +// label formats a candidate's display string: " () — +// " for ordinary interfaces. Loopback drops the "()" -- it's +// always "lo" and the friendly name already says "(testing)" -- giving +// "Loopback (testing) — 127.0.0.1". +func label(name, ipv4 string, loopback bool) string { + friendly := friendlyName(name) + if loopback { + return fmt.Sprintf("%s — %s", friendly, ipv4) + } + return fmt.Sprintf("%s (%s) — %s", friendly, name, ipv4) +} diff --git a/gui/internal/netpick/netpick_test.go b/gui/internal/netpick/netpick_test.go new file mode 100644 index 0000000..23d6647 --- /dev/null +++ b/gui/internal/netpick/netpick_test.go @@ -0,0 +1,174 @@ +package netpick + +import ( + "errors" + "net" + "testing" +) + +// realLoopback returns the host's real loopback interface. Every supported +// test environment (Linux CI, dev boxes) has one up with an IPv4 address, +// so tests use it as a genuine, resolvable Candidates() input instead of a +// synthetic net.Interface whose Addrs() would have nothing real to query. +func realLoopback(t *testing.T) net.Interface { + t.Helper() + ifaces, err := net.Interfaces() + if err != nil { + t.Fatalf("net.Interfaces(): %v", err) + } + for _, ifc := range ifaces { + if ifc.Flags&net.FlagLoopback != 0 && ifc.Flags&net.FlagUp != 0 { + return ifc + } + } + t.Skip("no up loopback interface on this host") + return net.Interface{} +} + +func TestCandidatesExcludesDownInterfaces(t *testing.T) { + lo := realLoopback(t) + down := net.Interface{Name: "down0", Flags: 0} + + got := Candidates(func() ([]net.Interface, error) { + return []net.Interface{lo, down}, nil + }) + + if len(got) != 1 || got[0].Name != lo.Name { + t.Fatalf("Candidates = %+v, want only %q", got, lo.Name) + } +} + +func TestCandidatesExcludesUpInterfacesWithoutIPv4(t *testing.T) { + lo := realLoopback(t) + // A nonexistent interface: Up but not resolvable to any address, so + // ifc.Addrs() fails and it must be excluded. + noAddr := net.Interface{Name: "nonexistent999", Index: 999999, Flags: net.FlagUp} + + got := Candidates(func() ([]net.Interface, error) { + return []net.Interface{lo, noAddr}, nil + }) + + if len(got) != 1 || got[0].Name != lo.Name { + t.Fatalf("Candidates = %+v, want only %q", got, lo.Name) + } +} + +func TestCandidatesReturnsNilOnListError(t *testing.T) { + got := Candidates(func() ([]net.Interface, error) { + return nil, errors.New("boom") + }) + if got != nil { + t.Errorf("Candidates = %+v, want nil", got) + } +} + +func TestCandidatesLabelsLoopback(t *testing.T) { + lo := realLoopback(t) + + got := Candidates(func() ([]net.Interface, error) { + return []net.Interface{lo}, nil + }) + + if len(got) != 1 { + t.Fatalf("Candidates = %+v, want exactly one candidate", got) + } + c := got[0] + if !c.Loopback { + t.Errorf("Candidate.Loopback = false, want true") + } + want := "Loopback (testing) — " + c.IPv4 + if c.Label != want { + t.Errorf("Candidate.Label = %q, want %q", c.Label, want) + } + if c.IPv4 == "" { + t.Errorf("Candidate.IPv4 = empty, want a dotted IPv4 address") + } +} + +func TestSortCandidatesOrdersNonLoopbackFirstThenName(t *testing.T) { + cands := []Candidate{ + {Name: "lo", Loopback: true}, + {Name: "wlan0", Loopback: false}, + {Name: "eno0", Loopback: false}, + } + + sortCandidates(cands) + + want := []string{"eno0", "wlan0", "lo"} + for i, name := range want { + if cands[i].Name != name { + t.Fatalf("sortCandidates order = %v, want %v", namesOf(cands), want) + } + } +} + +func namesOf(cands []Candidate) []string { + out := make([]string, len(cands)) + for i, c := range cands { + out[i] = c.Name + } + return out +} + +func TestLabelFormatsNonLoopback(t *testing.T) { + cases := []struct { + name string + want string + }{ + {"wlan0", "Wi-Fi (wlan0) — 10.0.0.5"}, + {"en0", "Wired (en0) — 10.0.0.5"}, + {"eth0", "Wired (eth0) — 10.0.0.5"}, + {"tun0", "Network (tun0) — 10.0.0.5"}, + } + for _, tc := range cases { + got := label(tc.name, "10.0.0.5", false) + if got != tc.want { + t.Errorf("label(%q, ...) = %q, want %q", tc.name, got, tc.want) + } + } +} + +func TestLabelLoopbackOmitsName(t *testing.T) { + got := label("lo", "127.0.0.1", true) + want := "Loopback (testing) — 127.0.0.1" + if got != want { + t.Errorf("label(loopback) = %q, want %q", got, want) + } +} + +func TestAutomaticPrefersFirstNonLoopback(t *testing.T) { + cands := []Candidate{ + {Name: "lo", Loopback: true}, + {Name: "eno0", Loopback: false}, + {Name: "wlan0", Loopback: false}, + } + + got, ok := Automatic(cands) + if !ok { + t.Fatal("Automatic returned ok = false, want true") + } + if got.Name != "eno0" { + t.Errorf("Automatic = %+v, want eno0", got) + } +} + +func TestAutomaticFallsBackToLoopback(t *testing.T) { + cands := []Candidate{ + {Name: "lo", Loopback: true}, + } + + got, ok := Automatic(cands) + if !ok { + t.Fatal("Automatic returned ok = false, want true") + } + if got.Name != "lo" { + t.Errorf("Automatic = %+v, want lo", got) + } +} + +func TestAutomaticReturnsFalseWhenEmpty(t *testing.T) { + _, ok := Automatic(nil) + if ok { + t.Error("Automatic(nil) ok = true, want false") + } +} diff --git a/gui/internal/session/bootstrap.go b/gui/internal/session/bootstrap.go index 38e3416..974e213 100644 --- a/gui/internal/session/bootstrap.go +++ b/gui/internal/session/bootstrap.go @@ -1,5 +1,20 @@ package session +import ( + "errors" + "net" + + "github.com/zyra/gobac/gui/internal/netpick" +) + +// resolveAutomatic picks the interface to use when the user has left the +// network setting on "Automatic": the best candidate netpick finds among +// the real OS interfaces. It's a func-var seam so bootstrap tests can +// substitute a fake candidate set without touching real network interfaces. +var resolveAutomatic = func() (netpick.Candidate, bool) { + return netpick.Automatic(netpick.Candidates(net.Interfaces)) +} + // Starter is the subset of Session that application startup/shutdown needs: // just enough to start a session from persisted settings and stop it again // on exit. Live satisfies it; tests substitute a fake so this wiring is @@ -22,7 +37,19 @@ func ConfigFromSettings(iface string, port int) Config { // are expected to surface that error non-fatally, e.g. in a status bar, // rather than aborting launch: other views such as the simulator quickstart // and scenario editor don't depend on a running session. +// +// An empty iface means "Automatic": it resolves to the best real interface +// via resolveAutomatic before starting, failing with a plain "no usable +// network found" if none is available. A non-empty iface passes through +// unchanged. func StartFromSettings(s Starter, iface string, port int) error { + if iface == "" { + c, ok := resolveAutomatic() + if !ok { + return errors.New("no usable network found") + } + iface = c.Name + } return s.Start(ConfigFromSettings(iface, port)) } diff --git a/gui/internal/session/bootstrap_test.go b/gui/internal/session/bootstrap_test.go index 91394b6..6fc1e09 100644 --- a/gui/internal/session/bootstrap_test.go +++ b/gui/internal/session/bootstrap_test.go @@ -3,6 +3,8 @@ package session import ( "errors" "testing" + + "github.com/zyra/gobac/gui/internal/netpick" ) // fakeStarter is a minimal Starter fake recording Start/Stop calls without @@ -62,6 +64,63 @@ func TestStartFromSettingsPropagatesError(t *testing.T) { } } +// withResolveAutomatic temporarily replaces the resolveAutomatic seam, +// restoring the original on cleanup. +func withResolveAutomatic(t *testing.T, fn func() (netpick.Candidate, bool)) { + t.Helper() + orig := resolveAutomatic + resolveAutomatic = fn + t.Cleanup(func() { resolveAutomatic = orig }) +} + +func TestStartFromSettingsResolvesEmptyInterfaceToAutomaticPick(t *testing.T) { + withResolveAutomatic(t, func() (netpick.Candidate, bool) { + return netpick.Candidate{Name: "fake0"}, true + }) + f := &fakeStarter{} + + if err := StartFromSettings(f, "", 47808); err != nil { + t.Fatalf("StartFromSettings: %v", err) + } + + want := Config{Interface: "fake0", Port: 47808, LocalPort: 47808} + if f.startCfg != want { + t.Errorf("Start called with %+v, want %+v", f.startCfg, want) + } +} + +func TestStartFromSettingsErrorsWhenNoCandidates(t *testing.T) { + withResolveAutomatic(t, func() (netpick.Candidate, bool) { + return netpick.Candidate{}, false + }) + f := &fakeStarter{} + + err := StartFromSettings(f, "", 47808) + if err == nil || err.Error() != "no usable network found" { + t.Fatalf("StartFromSettings error = %v, want %q", err, "no usable network found") + } + if f.startCalls != 0 { + t.Errorf("Start called %d times, want 0", f.startCalls) + } +} + +func TestStartFromSettingsPassesExplicitInterfaceUnchanged(t *testing.T) { + withResolveAutomatic(t, func() (netpick.Candidate, bool) { + t.Fatal("resolveAutomatic called for an explicit interface") + return netpick.Candidate{}, false + }) + f := &fakeStarter{} + + if err := StartFromSettings(f, "eno0", 47808); err != nil { + t.Fatalf("StartFromSettings: %v", err) + } + + want := Config{Interface: "eno0", Port: 47808, LocalPort: 47808} + if f.startCfg != want { + t.Errorf("Start called with %+v, want %+v", f.startCfg, want) + } +} + func TestShutdownStopsStarter(t *testing.T) { f := &fakeStarter{} diff --git a/gui/internal/ui/settings.go b/gui/internal/ui/settings.go index a92b695..9c440af 100644 --- a/gui/internal/ui/settings.go +++ b/gui/internal/ui/settings.go @@ -2,13 +2,21 @@ package ui import ( "fmt" + "net" "strconv" "fyne.io/fyne/v2" "fyne.io/fyne/v2/dialog" "fyne.io/fyne/v2/widget" + + "github.com/zyra/gobac/gui/internal/netpick" ) +// automaticLabel is the network Select's first option: choosing it persists +// an empty Settings.Interface, which StartFromSettings resolves to the best +// real interface at start time. +const automaticLabel = "Automatic (recommended)" + // Preference keys under app.Preferences(). const ( prefKeyInterface = "iface" @@ -71,39 +79,84 @@ func trySaveSettings(a fyne.App, iface, portText string) error { return nil } -// NewSettingsDialog builds the "Settings…" dialog: Interface and UDP Port -// fields, seeded from the current preferences, saving on confirm. -func NewSettingsDialog(a fyne.App, w fyne.Window) dialog.Dialog { +// networkOptions returns the network Select's option strings (Automatic +// first, then each netpick candidate's label) plus a lookup from label back +// to the interface name that should be persisted ("" for Automatic). +func networkOptions() (options []string, nameByLabel map[string]string) { + cands := netpick.Candidates(net.Interfaces) + + options = make([]string, 0, len(cands)+1) + options = append(options, automaticLabel) + nameByLabel = map[string]string{automaticLabel: ""} + + for _, c := range cands { + options = append(options, c.Label) + nameByLabel[c.Label] = c.Name + } + + return options, nameByLabel +} + +// labelForInterface returns the Select label that corresponds to iface +// (an interface name as persisted in Settings), falling back to +// automaticLabel when iface is empty or unrecognized (e.g. an interface +// that has since disappeared). +func labelForInterface(iface string) string { + if iface == "" { + return automaticLabel + } + cands := netpick.Candidates(net.Interfaces) + for _, c := range cands { + if c.Name == iface { + return c.Label + } + } + return automaticLabel +} + +// NewSettingsDialog builds the "Settings…" dialog: Network and Port fields, +// seeded from the current preferences, saving on confirm. restart, if +// non-nil, is called with the newly persisted Settings after a successful +// save so the caller can restart the session against the new network +// choice; it is not called when validation fails or the user cancels. +func NewSettingsDialog(a fyne.App, w fyne.Window, restart func(Settings)) dialog.Dialog { current := LoadSettings(a) - ifaceEntry := widget.NewEntry() - ifaceEntry.SetText(current.Interface) + options, nameByLabel := networkOptions() + + networkSelect := widget.NewSelect(options, nil) + networkSelect.SetSelected(labelForInterface(current.Interface)) portEntry := widget.NewEntry() portEntry.SetText(strconv.Itoa(current.Port)) portEntry.Validator = validatePort form := widget.NewForm( - widget.NewFormItem("Interface", ifaceEntry), - widget.NewFormItem("UDP Port", portEntry), + widget.NewFormItem("Network", networkSelect), + widget.NewFormItem("Port", portEntry), ) return dialog.NewCustomConfirm("Settings…", "Save", "Cancel", form, func(ok bool) { if !ok { return } - if err := trySaveSettings(a, ifaceEntry.Text, portEntry.Text); err != nil { + iface := nameByLabel[networkSelect.Selected] + if err := trySaveSettings(a, iface, portEntry.Text); err != nil { dialog.ShowError(err, w) return } + if restart != nil { + restart(LoadSettings(a)) + } }, w) } // NewMainMenu builds the application main menu, including the "Settings…" -// item that opens the settings dialog. -func NewMainMenu(a fyne.App, w fyne.Window) *fyne.MainMenu { +// item that opens the settings dialog. restart is forwarded to +// NewSettingsDialog (see its doc comment). +func NewMainMenu(a fyne.App, w fyne.Window, restart func(Settings)) *fyne.MainMenu { settingsItem := fyne.NewMenuItem("Settings…", func() { - NewSettingsDialog(a, w).Show() + NewSettingsDialog(a, w, restart).Show() }) fileMenu := fyne.NewMenu("File", settingsItem) return fyne.NewMainMenu(fileMenu) diff --git a/gui/internal/ui/settings_test.go b/gui/internal/ui/settings_test.go index f7ad9b4..ebb9afc 100644 --- a/gui/internal/ui/settings_test.go +++ b/gui/internal/ui/settings_test.go @@ -3,7 +3,9 @@ package ui import ( "testing" + "fyne.io/fyne/v2" "fyne.io/fyne/v2/test" + "fyne.io/fyne/v2/widget" ) func TestValidatePortRejectsInvalidValues(t *testing.T) { @@ -60,3 +62,208 @@ func TestTrySaveSettingsPersistsValidPort(t *testing.T) { t.Errorf("LoadSettings(a) = %+v, want Interface %q Port %d", got, "eno0", 47809) } } + +// TestNewSettingsDialogRendersAutomaticFirst renders the real dialog (not +// just its constructor return value) under the Fyne test driver and reads +// the actual rendered widget.Select: its first option must be "Automatic +// (recommended)", and with no interface saved yet that must be the +// pre-selected value too. +func TestNewSettingsDialogRendersAutomaticFirst(t *testing.T) { + a := test.NewApp() + w := a.NewWindow("t") + defer w.Close() + w.Resize(fyne.NewSize(400, 300)) + + NewSettingsDialog(a, w, nil).Show() + + sel := findSelect(w.Canvas().Overlays().Top()) + if sel == nil { + t.Fatal("no widget.Select found in rendered settings dialog") + } + if len(sel.Options) == 0 || sel.Options[0] != automaticLabel { + t.Fatalf("Select.Options = %v, want first entry %q", sel.Options, automaticLabel) + } + if sel.Selected != automaticLabel { + t.Errorf("Select.Selected = %q, want %q (no interface saved yet)", sel.Selected, automaticLabel) + } +} + +// TestNewSettingsDialogSavesSelectedInterfaceAndRestarts drives the +// rendered Select and Port entry, taps the real "Save" button found in the +// dialog's rendered object tree, and asserts on the outcome: the injected +// restart callback fires with the persisted Settings, and prefs hold the +// chosen interface's name (not its label, which embeds a volatile IP). +func TestNewSettingsDialogSavesSelectedInterfaceAndRestarts(t *testing.T) { + a := test.NewApp() + w := a.NewWindow("t") + defer w.Close() + w.Resize(fyne.NewSize(400, 300)) + + var restarted Settings + restartCalls := 0 + NewSettingsDialog(a, w, func(s Settings) { + restartCalls++ + restarted = s + }).Show() + + top := w.Canvas().Overlays().Top() + + sel := findSelect(top) + if sel == nil { + t.Fatal("no widget.Select found in rendered settings dialog") + } + if len(sel.Options) < 2 { + t.Fatal("expected at least one real candidate besides Automatic; the host's loopback interface should always qualify") + } + // The candidate list sorts non-loopback first, loopback last, so the + // last option is always present and deterministic across hosts. + chosen := sel.Options[len(sel.Options)-1] + sel.SetSelected(chosen) + + entry := findEntry(top) + if entry == nil { + t.Fatal("no widget.Entry found in rendered settings dialog") + } + entry.SetText("47809") + + save := findButton(top, "Save") + if save == nil { + t.Fatal("no \"Save\" button found in rendered settings dialog") + } + test.Tap(save) + + if restartCalls != 1 { + t.Fatalf("restart called %d times, want 1", restartCalls) + } + if restarted.Port != 47809 { + t.Errorf("restart Settings.Port = %d, want 47809", restarted.Port) + } + if restarted.Interface == "" { + t.Errorf("restart Settings.Interface = empty, want the chosen candidate's interface name") + } + + got := LoadSettings(a) + if got.Port != 47809 || got.Interface != restarted.Interface { + t.Errorf("LoadSettings(a) = %+v, want Port 47809 Interface %q", got, restarted.Interface) + } +} + +// TestNewSettingsDialogInvalidPortShowsErrorAndSkipsSaveAndRestart taps the +// rendered "Save" button with an out-of-range port and asserts the outcome +// on rendered state: a new overlay (the error dialog) appears on the +// canvas, nothing was persisted, and restart never fired. +func TestNewSettingsDialogInvalidPortShowsErrorAndSkipsSaveAndRestart(t *testing.T) { + a := test.NewApp() + w := a.NewWindow("t") + defer w.Close() + w.Resize(fyne.NewSize(400, 300)) + + SaveSettings(a, Settings{Interface: "", Port: DefaultPort}) + + restartCalls := 0 + NewSettingsDialog(a, w, func(Settings) { restartCalls++ }).Show() + + top := w.Canvas().Overlays().Top() + + entry := findEntry(top) + if entry == nil { + t.Fatal("no widget.Entry found in rendered settings dialog") + } + entry.SetText("70000") + + save := findButton(top, "Save") + if save == nil { + t.Fatal("no \"Save\" button found in rendered settings dialog") + } + test.Tap(save) + + // A successful Save hides the settings dialog's own popup before + // calling back; an invalid port instead pushes a new error dialog over + // it, so the canvas's top overlay must have changed to something new. + newTop := w.Canvas().Overlays().Top() + if newTop == nil || newTop == top { + t.Fatal("no new overlay appeared after an invalid-port Save; want an error dialog rendered on top") + } + + if restartCalls != 0 { + t.Errorf("restart called %d times after invalid port, want 0", restartCalls) + } + got := LoadSettings(a) + if got.Port != DefaultPort { + t.Errorf("LoadSettings(a).Port = %d, want unchanged %d", got.Port, DefaultPort) + } +} + +// walkCanvasObject recurses into obj -- through *fyne.Container.Objects and, +// for any fyne.Widget, its rendered test.WidgetRenderer(obj).Objects() -- +// calling visit on every object reached (including obj itself) until visit +// reports a match. +func walkCanvasObject(obj fyne.CanvasObject, visit func(fyne.CanvasObject) bool) bool { + if obj == nil { + return false + } + if visit(obj) { + return true + } + if c, ok := obj.(*fyne.Container); ok { + for _, child := range c.Objects { + if walkCanvasObject(child, visit) { + return true + } + } + return false + } + if wid, ok := obj.(fyne.Widget); ok { + r := test.WidgetRenderer(wid) + if r == nil { + return false + } + for _, child := range r.Objects() { + if walkCanvasObject(child, visit) { + return true + } + } + } + return false +} + +// findSelect returns the first *widget.Select reachable from root. +func findSelect(root fyne.CanvasObject) *widget.Select { + var found *widget.Select + walkCanvasObject(root, func(o fyne.CanvasObject) bool { + s, ok := o.(*widget.Select) + if ok { + found = s + } + return ok + }) + return found +} + +// findEntry returns the first *widget.Entry reachable from root. +func findEntry(root fyne.CanvasObject) *widget.Entry { + var found *widget.Entry + walkCanvasObject(root, func(o fyne.CanvasObject) bool { + e, ok := o.(*widget.Entry) + if ok { + found = e + } + return ok + }) + return found +} + +// findButton returns the first *widget.Button reachable from root whose +// Text matches text. +func findButton(root fyne.CanvasObject, text string) *widget.Button { + var found *widget.Button + walkCanvasObject(root, func(o fyne.CanvasObject) bool { + b, ok := o.(*widget.Button) + if ok && b.Text == text { + found = b + return true + } + return false + }) + return found +} From f802050631f7afe48dbaa66de3685d07244b6b69 Mon Sep 17 00:00:00 2001 From: Ibby Hadeed Date: Sat, 18 Jul 2026 20:28:34 +0400 Subject: [PATCH 03/10] Consolidate Simulator Editor and Quickstart into one Simulator view Adds Run/Stop controls and a running-devices strip to the scenario editor, migrating the runner lifecycle from the deleted Quickstart view. Run validates the document, serializes it via the editor's own scenariodoc.Document, and starts it through simrun; a scenario simrun can't run gets plain-language guidance instead of the raw error. "Load example scenario" loads the bundled quickstart scenario into the editor (confirming first if there are unsaved edits), so the old one-click demo becomes Simulator -> Load example scenario -> Run. Injected devices carry Source "simulated" and a Name, so Discovery can tell them apart from real Who-Is sightings and show their name; the DeviceStore merges Name the same way it already preserves Source across re-sightings. Discovery gains a Name column and renders Source as plain-language "Simulated"/"Network" text. boot.go wires the shared DeviceStore into the Simulator view and a PortHint callback so a running simulation's status includes a tip when the current session port won't reach it. --- gui/internal/boot/boot.go | 45 ++- gui/internal/boot/boot_test.go | 37 +++ gui/internal/scenariodoc/document.go | 13 + gui/internal/scenariodoc/document_test.go | 39 +++ gui/internal/simrun/runner.go | 19 +- gui/internal/simrun/runner_test.go | 6 +- gui/internal/store/devices.go | 34 ++- gui/internal/store/devices_test.go | 50 ++- gui/internal/ui/discovery.go | 26 +- gui/internal/ui/discovery_test.go | 62 +++- gui/internal/ui/editor.go | 351 ++++++++++++++++++++-- gui/internal/ui/editor_test.go | 285 +++++++++++++++++- gui/internal/ui/quickstart.go | 220 -------------- gui/internal/ui/quickstart_test.go | 169 ----------- gui/internal/ui/shell.go | 8 +- gui/internal/ui/shell_test.go | 6 +- 16 files changed, 894 insertions(+), 476 deletions(-) delete mode 100644 gui/internal/ui/quickstart.go delete mode 100644 gui/internal/ui/quickstart_test.go diff --git a/gui/internal/boot/boot.go b/gui/internal/boot/boot.go index d51cfaa..068818a 100644 --- a/gui/internal/boot/boot.go +++ b/gui/internal/boot/boot.go @@ -16,14 +16,15 @@ import ( "github.com/zyra/gobac/gui/internal/ui" ) -// discoveryNavIndex, browserNavIndex, editorNavIndex, and quickstartNavIndex -// are the AppShell nav indices of the Discovery, Object Browser, Simulator -// Editor, and Quickstart views (see navLabels in internal/ui/shell.go). +// discoveryNavIndex, browserNavIndex, and editorNavIndex are the AppShell +// nav indices of the Discovery, Object Browser, and Simulator views (see +// navLabels in internal/ui/shell.go). Task U3 folded the former Quickstart +// view into the Simulator (editor) view, so there is no separate nav index +// for it anymore. const ( - discoveryNavIndex = 0 - browserNavIndex = 1 - editorNavIndex = 2 - quickstartNavIndex = 3 + discoveryNavIndex = 0 + browserNavIndex = 1 + editorNavIndex = 2 ) // Compose builds the application shell and wires it to sess: window sizing @@ -61,11 +62,13 @@ func Compose(a fyne.App, w fyne.Window, sess session.Session) *ui.AppShell { browser := ui.NewBrowserView(sess, objects, shell) shell.SetView(browserNavIndex, browser) - editor := ui.NewEditorView(shell) + editor := ui.NewEditorView(devices, shell) shell.SetView(editorNavIndex, editor) - - quickstart := ui.NewQuickstartView(devices, shell) - shell.SetView(quickstartNavIndex, quickstart) + if editorView, ok := editor.(*ui.EditorView); ok { + editorView.PortHint = func(ports []uint16) string { + return sessionPortHint(a, ports) + } + } if discoveryView, ok := discovery.(*ui.DiscoveryView); ok { if browserView, ok := browser.(*ui.BrowserView); ok { @@ -95,6 +98,26 @@ func startSession(sess session.Session, shell *ui.AppShell, s ui.Settings) { shell.SetStatus(fmt.Sprintf("Connected on %s (port %d)", label, s.Port)) } +// sessionPortHint implements EditorView.PortHint: it reports whether any of +// a just-started simulation's ports matches the session's currently +// configured port, and if not, returns a plain-language tip naming the +// first running port so the user knows what to change Settings -> Port to +// in order to reach these simulated devices through the Network Explorer +// browser. Returns "" (no hint) when there is a match or no running +// devices at all. +func sessionPortHint(a fyne.App, ports []uint16) string { + if len(ports) == 0 { + return "" + } + settings := ui.LoadSettings(a) + for _, p := range ports { + if int(p) == settings.Port { + return "" + } + } + return fmt.Sprintf("Tip: set Settings → Port to %d to interact with these devices.", ports[0]) +} + // interfaceLabel returns the human-friendly netpick label for iface (a // Settings.Interface value): the Automatic pick's label when iface is // empty, or the matching candidate's label for a named interface. It falls diff --git a/gui/internal/boot/boot_test.go b/gui/internal/boot/boot_test.go index c73a195..8856046 100644 --- a/gui/internal/boot/boot_test.go +++ b/gui/internal/boot/boot_test.go @@ -100,6 +100,43 @@ func TestComposeReportsFailureStatusWhenStartFails(t *testing.T) { } } +// TestSessionPortHintMatchesRunningPortReturnsEmpty covers sessionPortHint's +// no-hint case: the session's configured port already matches one of the +// simulation's running ports, so no Settings tip is needed. +func TestSessionPortHintMatchesRunningPortReturnsEmpty(t *testing.T) { + a := test.NewApp() + ui.SaveSettings(a, ui.Settings{Interface: "eno0", Port: 47902}) + + got := sessionPortHint(a, []uint16{47901, 47902}) + if got != "" { + t.Errorf("sessionPortHint = %q, want \"\" (session port matches a running port)", got) + } +} + +// TestSessionPortHintMismatchReturnsTipNamingFirstPort covers the actual +// tip: no running port matches the session's configured port, so the hint +// names the first running port. +func TestSessionPortHintMismatchReturnsTipNamingFirstPort(t *testing.T) { + a := test.NewApp() + ui.SaveSettings(a, ui.Settings{Interface: "eno0", Port: 47808}) + + got := sessionPortHint(a, []uint16{47901, 47902}) + want := "Tip: set Settings → Port to 47901 to interact with these devices." + if got != want { + t.Errorf("sessionPortHint = %q, want %q", got, want) + } +} + +// TestSessionPortHintNoRunningDevicesReturnsEmpty covers the degenerate +// zero-ports case (should never happen in practice — Run always injects at +// least the devices it started — but must not panic or fabricate a tip). +func TestSessionPortHintNoRunningDevicesReturnsEmpty(t *testing.T) { + a := test.NewApp() + if got := sessionPortHint(a, nil); got != "" { + t.Errorf("sessionPortHint(nil) = %q, want \"\"", got) + } +} + // TestComposedWindowRendersNonBlank exercises the exact composition main() // performs (via Compose) under the Fyne test driver and asserts on the // rendered pixels, not the container tree. Before AppShell became a proper diff --git a/gui/internal/scenariodoc/document.go b/gui/internal/scenariodoc/document.go index 120473e..43aa077 100644 --- a/gui/internal/scenariodoc/document.go +++ b/gui/internal/scenariodoc/document.go @@ -65,6 +65,19 @@ func Load(path string) (*Document, error) { return &Document{path: path, format: format, scenario: *scenario}, nil } +// LoadBytes decodes a scenario from data (format rules match Load's file +// extension mapping — pass "yaml" or "json" directly) into a new Document +// with no destination path. Used to load a bundled scenario (e.g. the +// Simulator view's example scenario) without first writing it to disk; the +// returned Document is not dirty, exactly like one just opened with Load. +func LoadBytes(data []byte, format string) (*Document, error) { + scenario, err := simulator.DecodeScenario(bytes.NewReader(data), format) + if err != nil { + return nil, err + } + return &Document{format: format, scenario: *scenario}, nil +} + // formatForPath derives a DecodeScenario/marshal format from a file // extension: the trimmed, lowercased extension (e.g. "yaml", "yml", "json", // or "" for an extensionless path) — mirroring cmd/gobac-sim/main.go. diff --git a/gui/internal/scenariodoc/document_test.go b/gui/internal/scenariodoc/document_test.go index b98989e..6eb80df 100644 --- a/gui/internal/scenariodoc/document_test.go +++ b/gui/internal/scenariodoc/document_test.go @@ -32,6 +32,45 @@ func TestLoadSaveReloadRoundTrips(t *testing.T) { } } +// TestLoadBytesDecodesWithNoDestinationPath covers the Simulator view's +// "Load example scenario" path (task U3): decoding an embedded scenario's +// bytes must produce the same scenario Load would from the equivalent file, +// but with no destination path and no dirty flag. +func TestLoadBytesDecodesWithNoDestinationPath(t *testing.T) { + data, err := os.ReadFile("testdata/roundtrip.yaml") + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + + doc, err := LoadBytes(data, "yaml") + if err != nil { + t.Fatalf("LoadBytes: %v", err) + } + + loaded, err := Load("testdata/roundtrip.yaml") + if err != nil { + t.Fatalf("Load: %v", err) + } + if !reflect.DeepEqual(doc.Scenario(), loaded.Scenario()) { + t.Fatalf("LoadBytes scenario mismatch:\ngot: %#v\nwant: %#v", doc.Scenario(), loaded.Scenario()) + } + if doc.Path() != "" { + t.Errorf("Path() = %q, want \"\" (LoadBytes has no destination path)", doc.Path()) + } + if doc.Dirty() { + t.Error("Dirty() = true for a freshly decoded document, want false") + } +} + +// TestLoadBytesRejectsInvalidScenario covers the decode-failure path: bytes +// that don't decode into a valid scenario return an error rather than a +// document holding a zero-value scenario. +func TestLoadBytesRejectsInvalidScenario(t *testing.T) { + if _, err := LoadBytes([]byte("not valid scenario yaml: [1, 2"), "yaml"); err == nil { + t.Fatal("expected an error decoding invalid scenario bytes, got nil") + } +} + func TestNewDocumentValidatesAndSaves(t *testing.T) { doc := New() if err := doc.Validate(); err != nil { diff --git a/gui/internal/simrun/runner.go b/gui/internal/simrun/runner.go index 396a1bf..c0acf53 100644 --- a/gui/internal/simrun/runner.go +++ b/gui/internal/simrun/runner.go @@ -1,6 +1,7 @@ // Package simrun runs a simulator.Scenario's devices in-process, each as a -// loopback UDP BACnet/IP responder, for the Quickstart view (task G8). It -// has no dependency on Fyne and is unit-tested on its own. +// loopback UDP BACnet/IP responder, for the Simulator view's Run/Stop +// controls (originally task G8, moved into the Simulator view by task U3). +// It has no dependency on Fyne and is unit-tested on its own. package simrun import ( @@ -15,9 +16,11 @@ import ( "github.com/zyra/gobac/v2/simulator" ) -// errUnsupportedScenario is returned by Start when sc is not a loopback -// multi-port (or single-device) scenario. -var errUnsupportedScenario = errors.New("quickstart requires multi-port loopback scenarios") +// ErrUnsupportedScenario is returned by Start when sc is not a loopback +// multi-port (or single-device) scenario. Exported so callers (the +// Simulator view) can recognize it with errors.Is and show plain-language +// guidance instead of the raw error text. +var ErrUnsupportedScenario = errors.New("the simulator requires multi-port loopback scenarios") // deviceIP is the loopback address every in-process device binds to. It // deliberately is not 127.0.0.1: the wrapped client library always sends a @@ -57,7 +60,7 @@ type Runner struct { // Start validates sc, builds its object model, and brings up one loopback // UDP responder per device, each serving on a goroutine. It requires a // loopback multi-port (or single-device) scenario with no non-loopback -// device addresses: quickstart runs are scoped to loopback so they can +// device addresses: simulator runs are scoped to loopback so they can // never collide with, or be mistaken for, a real BACnet/IP network // (gui-architecture.md §4.5). // @@ -68,11 +71,11 @@ func Start(ctx context.Context, sc *simulator.Scenario) (*Runner, error) { return nil, errors.New("scenario is nil") } if sc.Network.Mode != "multi-port" && sc.Network.Mode != "single-device" { - return nil, errUnsupportedScenario + return nil, ErrUnsupportedScenario } for i := range sc.Devices { if !isLoopbackOrEmpty(sc.Devices[i].Address) { - return nil, errUnsupportedScenario + return nil, ErrUnsupportedScenario } } diff --git a/gui/internal/simrun/runner_test.go b/gui/internal/simrun/runner_test.go index 28f2894..ade1ad1 100644 --- a/gui/internal/simrun/runner_test.go +++ b/gui/internal/simrun/runner_test.go @@ -3,8 +3,8 @@ package simrun import ( "bytes" "context" + "errors" "net" - "strings" "testing" "time" @@ -183,7 +183,7 @@ func TestStartRejectsMultiIPScenario(t *testing.T) { if err == nil { t.Fatal("expected an error for a multi-ip scenario, got nil") } - if !strings.Contains(err.Error(), "quickstart requires multi-port loopback scenarios") { - t.Fatalf("error = %q, want it to contain %q", err.Error(), "quickstart requires multi-port loopback scenarios") + if !errors.Is(err, ErrUnsupportedScenario) { + t.Fatalf("error = %v, want it to wrap ErrUnsupportedScenario", err) } } diff --git a/gui/internal/store/devices.go b/gui/internal/store/devices.go index 4b490e7..69dd15a 100644 --- a/gui/internal/store/devices.go +++ b/gui/internal/store/devices.go @@ -21,9 +21,13 @@ type DeviceRow struct { MaxApdu uint32 Segmentation uint8 // Source records where this row came from: "network" for a real - // Who-Is sighting, "local-sim" for a device injected by the - // in-process simulator quickstart (G8). - Source string + // Who-Is sighting, "simulated" for a device injected by the Simulator + // view's in-process runner (task U3). + Source string + // Name is the device's scenario name when known (every "simulated" + // row has one); "" for a "network" row, since a real Who-Is sighting + // never carries a name. + Name string LastSeen time.Time } @@ -52,14 +56,26 @@ func NewDeviceStore() *DeviceStore { // Upsert inserts or merges row into the store, stamping LastSeen from // Now(), and notifies listeners. // -// A row already recorded with Source "local-sim" keeps that Source even -// when re-sighted with Source "network" — a quickstart device stays -// identified as local-sim even if it also answers a real Who-Is sweep on -// loopback. Every other field is replaced with the incoming row's value. +// Two fields merge with the existing row instead of being blindly +// overwritten: +// - A row already recorded with Source "simulated" keeps that Source +// even when re-sighted with Source "network" — a simulated device +// stays identified as simulated even if it also answers a real Who-Is +// sweep on loopback. +// - Name is kept from the existing row when the incoming row's Name is +// empty, so a simulated device's name survives a later re-sighting +// from a source (like a Who-Is sweep) that never carries one. +// +// Every other field is replaced with the incoming row's value. func (s *DeviceStore) Upsert(row DeviceRow) { s.mu.Lock() - if existing, ok := s.rows[row.Key]; ok && existing.Source == "local-sim" && row.Source == "network" { - row.Source = "local-sim" + if existing, ok := s.rows[row.Key]; ok { + if existing.Source == "simulated" && row.Source == "network" { + row.Source = "simulated" + } + if row.Name == "" && existing.Name != "" { + row.Name = existing.Name + } } if s.Now != nil { row.LastSeen = s.Now() diff --git a/gui/internal/store/devices_test.go b/gui/internal/store/devices_test.go index b1f5cca..26e643f 100644 --- a/gui/internal/store/devices_test.go +++ b/gui/internal/store/devices_test.go @@ -106,19 +106,61 @@ func TestDeviceStoreConcurrentUpserts(t *testing.T) { } } -func TestDeviceStoreSourceStaysLocalSim(t *testing.T) { +func TestDeviceStoreSourceStaysSimulated(t *testing.T) { s := NewDeviceStore() key := DeviceKey{Instance: 1001, IP: "127.0.0.1"} - s.Upsert(DeviceRow{Key: key, Source: "local-sim"}) + s.Upsert(DeviceRow{Key: key, Source: "simulated"}) s.Upsert(DeviceRow{Key: key, Source: "network"}) rows := s.Snapshot() if len(rows) != 1 { t.Fatalf("Snapshot() len = %d, want 1", len(rows)) } - if rows[0].Source != "local-sim" { - t.Errorf("Source = %q, want %q", rows[0].Source, "local-sim") + if rows[0].Source != "simulated" { + t.Errorf("Source = %q, want %q", rows[0].Source, "simulated") + } +} + +// TestDeviceStoreNameSurvivesResightingWithoutName covers the Name merge +// behavior: a simulated device's Name must not be wiped out when the same +// key is later re-upserted (e.g. by a Who-Is sweep) with an empty Name. +func TestDeviceStoreNameSurvivesResightingWithoutName(t *testing.T) { + s := NewDeviceStore() + key := DeviceKey{Instance: 1001, IP: "127.0.0.2"} + + s.Upsert(DeviceRow{Key: key, Source: "simulated", Name: "Boiler"}) + s.Upsert(DeviceRow{Key: key, Source: "network", VendorID: 5}) + + rows := s.Snapshot() + if len(rows) != 1 { + t.Fatalf("Snapshot() len = %d, want 1", len(rows)) + } + if rows[0].Name != "Boiler" { + t.Errorf("Name = %q, want %q", rows[0].Name, "Boiler") + } + if rows[0].VendorID != 5 { + t.Errorf("VendorID = %d, want 5 (non-Name fields still overwrite)", rows[0].VendorID) + } +} + +// TestDeviceStoreNameOverwritesWhenIncomingHasOne covers the other half of +// the merge rule: an explicit incoming Name replaces whatever was there +// before (Name only falls back to the existing value when the incoming one +// is empty). +func TestDeviceStoreNameOverwritesWhenIncomingHasOne(t *testing.T) { + s := NewDeviceStore() + key := DeviceKey{Instance: 1001, IP: "127.0.0.2"} + + s.Upsert(DeviceRow{Key: key, Source: "simulated", Name: "Boiler"}) + s.Upsert(DeviceRow{Key: key, Source: "simulated", Name: "Renamed Boiler"}) + + rows := s.Snapshot() + if len(rows) != 1 { + t.Fatalf("Snapshot() len = %d, want 1", len(rows)) + } + if rows[0].Name != "Renamed Boiler" { + t.Errorf("Name = %q, want %q", rows[0].Name, "Renamed Boiler") } } diff --git a/gui/internal/ui/discovery.go b/gui/internal/ui/discovery.go index 60ad5ac..ad0f730 100644 --- a/gui/internal/ui/discovery.go +++ b/gui/internal/ui/discovery.go @@ -16,7 +16,7 @@ import ( // discoveryColumns are the discovery table's column headers, in display // order. var discoveryColumns = []string{ - "Instance", "Address", "Vendor ID", "Max APDU", "Segmentation", "Source", "Last seen", + "Instance", "Name", "Address", "Vendor ID", "Max APDU", "Segmentation", "Source", "Last seen", } // segmentationText maps types.Segmentation values (bacnet/types/segmentation.go) @@ -28,6 +28,15 @@ var segmentationText = map[uint8]string{ 3: "none", } +// sourceText maps store.DeviceRow.Source values to the plain-language +// display text the Source column renders (task U3): "Simulated" for a +// device injected by the Simulator view's Run controls, "Network" for a +// real Who-Is sighting. +var sourceText = map[string]string{ + "simulated": "Simulated", + "network": "Network", +} + // defaultSweepDuration is the pre-selected entry in the duration selector. const defaultSweepDuration = "3s" @@ -142,19 +151,24 @@ func (v *DiscoveryView) cellText(id widget.TableCellID) string { case 0: return fmt.Sprintf("%d", row.Key.Instance) case 1: - return fmt.Sprintf("%s:%d", row.Key.IP, row.Port) + return row.Name case 2: - return fmt.Sprintf("%d", row.VendorID) + return fmt.Sprintf("%s:%d", row.Key.IP, row.Port) case 3: - return fmt.Sprintf("%d", row.MaxApdu) + return fmt.Sprintf("%d", row.VendorID) case 4: + return fmt.Sprintf("%d", row.MaxApdu) + case 5: if text, ok := segmentationText[row.Segmentation]; ok { return text } return fmt.Sprintf("%d", row.Segmentation) - case 5: - return row.Source case 6: + if text, ok := sourceText[row.Source]; ok { + return text + } + return row.Source + case 7: return row.LastSeen.Format("15:04:05") } return "" diff --git a/gui/internal/ui/discovery_test.go b/gui/internal/ui/discovery_test.go index 4da27a0..7cd14e9 100644 --- a/gui/internal/ui/discovery_test.go +++ b/gui/internal/ui/discovery_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "fyne.io/fyne/v2" "fyne.io/fyne/v2/test" "fyne.io/fyne/v2/widget" @@ -147,7 +148,7 @@ func TestSweepPopulatesTableWithExactCellText(t *testing.T) { t.Fatalf("table cols = %d, want %d", cols, len(discoveryColumns)) } - want := []string{"70001", "192.0.2.10:47808", "260", "1476", "none", "network", "15:04:05"} + want := []string{"70001", "", "192.0.2.10:47808", "260", "1476", "none", "Network", "15:04:05"} for col, wantText := range want { if got := cellText(view, 0, col); got != wantText { t.Errorf("row 0 col %d (%s) = %q, want %q", col, discoveryColumns[col], got, wantText) @@ -256,3 +257,62 @@ func TestSelectingRowInvokesOnSelectWithExactRow(t *testing.T) { t.Errorf("OnSelect row = %+v, want %+v", got, want) } } + +// TestDiscoveryTableRendersSimulatedAndNetworkRows covers the U3 Source/Name +// columns end to end: a "simulated" row (with a Name) and a "network" row +// (without one) both appear in the rendered table, with the Source column +// showing plain-language "Simulated"/"Network" text and the Name column +// blank only for the row with no known name. +func TestDiscoveryTableRendersSimulatedAndNetworkRows(t *testing.T) { + fake := &fakeDiscoverSession{} + view, devices := newDiscoveryTestView(t, fake) + + a := test.NewApp() + w := a.NewWindow("capture") + defer w.Close() + w.SetContent(view) + w.Resize(fyne.NewSize(700, 300)) + + before := w.Canvas().Capture() + + devices.Upsert(store.DeviceRow{ + Key: store.DeviceKey{Instance: 1001, IP: "127.0.0.2"}, + Port: 47901, + Name: "Boiler", + Source: "simulated", + }) + devices.Upsert(store.DeviceRow{ + Key: store.DeviceKey{Instance: 70001, IP: "192.0.2.10"}, + Port: 47808, + VendorID: 260, + Source: "network", + }) + + after := w.Canvas().Capture() + if imagesEqual(before, after) { + t.Fatal("canvas capture is unchanged after upserting two device rows") + } + + rows, cols := view.table.Length() + if rows != 2 { + t.Fatalf("table rows = %d, want 2", rows) + } + if cols != len(discoveryColumns) { + t.Fatalf("table cols = %d, want %d", cols, len(discoveryColumns)) + } + + // Snapshot is sorted by Instance ascending: row 0 is the simulated + // device (1001), row 1 is the network device (70001). + if got, want := cellText(view, 0, 1), "Boiler"; got != want { + t.Errorf("row 0 (simulated) Name column = %q, want %q", got, want) + } + if got, want := cellText(view, 0, 6), "Simulated"; got != want { + t.Errorf("row 0 (simulated) Source column = %q, want %q", got, want) + } + if got, want := cellText(view, 1, 1), ""; got != want { + t.Errorf("row 1 (network) Name column = %q, want %q (blank when unknown)", got, want) + } + if got, want := cellText(view, 1, 6), "Network"; got != want { + t.Errorf("row 1 (network) Source column = %q, want %q", got, want) + } +} diff --git a/gui/internal/ui/editor.go b/gui/internal/ui/editor.go index c3b2210..865f432 100644 --- a/gui/internal/ui/editor.go +++ b/gui/internal/ui/editor.go @@ -1,6 +1,7 @@ package ui import ( + "context" "errors" "fmt" "strconv" @@ -12,7 +13,10 @@ import ( "fyne.io/fyne/v2/storage" "fyne.io/fyne/v2/widget" + "github.com/zyra/gobac/gui/assets" "github.com/zyra/gobac/gui/internal/scenariodoc" + "github.com/zyra/gobac/gui/internal/simrun" + "github.com/zyra/gobac/gui/internal/store" "github.com/zyra/gobac/v2/simulator" ) @@ -51,9 +55,27 @@ func classifyObjectType(t string) string { } } -// EditorView is the Simulator Editor navigation entry: toolbar (New, Open, -// Save, Save As), a network form, and master-detail device/object editing -// over a scenariodoc.Document, with live validation. +// simRunner is the subset of *simrun.Runner the Simulator view depends on, +// so tests can substitute a fake instead of running real UDP sockets. +type simRunner interface { + Devices() []simrun.RunningDevice + Stop() + Err() <-chan error +} + +var _ simRunner = (*simrun.Runner)(nil) + +// defaultStartRunner decodes sc and starts it via simrun.Start, adapting +// *simrun.Runner to the simRunner interface. +func defaultStartRunner(ctx context.Context, sc *simulator.Scenario) (simRunner, error) { + return simrun.Start(ctx, sc) +} + +// EditorView is the Simulator navigation entry (task U3 — consolidating the +// former Simulator Editor + Quickstart views): toolbar (New, Open, Save, +// Save As, Load example scenario, Run, Stop), a network form, master-detail +// device/object editing over a scenariodoc.Document with live validation, +// and a running-devices strip visible while a simulation is live. // // EditorView is a proper widget (widget.BaseWidget + CreateRenderer) rather // than an embedded *fyne.Container; see the identical note on DiscoveryView @@ -61,16 +83,56 @@ func classifyObjectType(t string) string { type EditorView struct { widget.BaseWidget - shell *AppShell - doc *scenariodoc.Document + shell *AppShell + devices *store.DeviceStore + doc *scenariodoc.Document // fieldErrors is the most recent scenariodoc.FieldErrors result, kept // for both the field-hint display and same-package test access. fieldErrors map[string]string - - titleLabel *widget.Label - summaryLabel *widget.Label - saveBtn *widget.Button + // valid mirrors the most recent Document.Validate() outcome; it gates + // the Run button the same way fieldErrors gates saveBtn (a document + // simrun could never run is never offered as runnable). + valid bool + + titleLabel *widget.Label + summaryLabel *widget.Label + saveBtn *widget.Button + loadExampleBtn *widget.Button + runBtn *widget.Button + stopBtn *widget.Button + + // runningRowsBox holds one Label per live device, rebuilt on every + // Run/Stop by refreshRunningRows. A plain VBox (rather than a + // widget.List, which is a scroller sized independently of its content) + // so the strip's rendered height always includes every row instead of + // clipping to the scroller's own small default MinSize. + runningRowsBox *fyne.Container + runningRows []simrun.RunningDevice + runningStrip *fyne.Container + + running simRunner + errWatchStop chan struct{} + + // StartRunner starts a decoded scenario. Exported so tests can replace + // it with a fake runner instead of exercising real UDP sockets. + // Defaults to wrapping simrun.Start. + StartRunner func(ctx context.Context, sc *simulator.Scenario) (simRunner, error) + + // PortHint, when set, is called after a simulation starts with the + // ports of every running device. A non-empty return value is appended + // (space-separated) to the "Simulation running" status text as an + // extra plain-language sentence. Wired by boot.go to surface a + // Settings-port mismatch; nil (no hint) by default. + PortHint func(ports []uint16) string + + // startDone/stopDone are test-only synchronization seams, mirroring + // DiscoveryView.sweepDone: if non-nil when the corresponding method is + // invoked, each is closed once that call's background goroutine + // finishes all of its work (including any fyne.Do UI update). + // Production code leaves them nil. + startDone chan struct{} + stopDone chan struct{} modeSelect *widget.Select ifaceEntry *widget.Entry @@ -115,15 +177,18 @@ type EditorView struct { root *fyne.Container } -// NewEditorView builds the Simulator Editor view, holding a -// *scenariodoc.Document that starts as scenariodoc.New() (a minimal, -// already-valid single-device scenario). -func NewEditorView(shell *AppShell) fyne.CanvasObject { +// NewEditorView builds the Simulator view, holding a *scenariodoc.Document +// that starts as scenariodoc.New() (a minimal, already-valid single-device +// scenario). devices is the shared DeviceStore that Run injects rows into +// (Source "simulated") and Stop removes them from. +func NewEditorView(devices *store.DeviceStore, shell *AppShell) fyne.CanvasObject { v := &EditorView{ shell: shell, + devices: devices, doc: scenariodoc.New(), selectedDevice: -1, selectedObject: -1, + StartRunner: defaultStartRunner, } v.buildWidgets() v.ExtendBaseWidget(v) @@ -146,7 +211,21 @@ func (v *EditorView) buildWidgets() { openBtn := widget.NewButton("Open", v.onOpen) v.saveBtn = widget.NewButton("Save", v.onSave) saveAsBtn := widget.NewButton("Save As", v.onSaveAs) - toolbar := container.NewHBox(newBtn, openBtn, v.saveBtn, saveAsBtn) + v.loadExampleBtn = widget.NewButton("Load example scenario", v.onLoadExample) + v.runBtn = widget.NewButton("▶ Run", v.onRun) + v.stopBtn = widget.NewButton("■ Stop", v.onStop) + v.stopBtn.Disable() + toolbar := container.NewHBox( + newBtn, openBtn, v.saveBtn, saveAsBtn, + widget.NewSeparator(), + v.loadExampleBtn, + widget.NewSeparator(), + v.runBtn, v.stopBtn, + ) + + v.runningRowsBox = container.NewVBox() + v.runningStrip = container.NewVBox(widget.NewLabel("Running devices:"), v.runningRowsBox) + v.runningStrip.Hide() v.titleLabel = widget.NewLabel("") v.summaryLabel = widget.NewLabel("valid") @@ -197,7 +276,24 @@ func (v *EditorView) buildWidgets() { mainSplit := container.NewHSplit(listSplit, formSplit) top := container.NewVBox(toolbar, v.titleLabel, networkForm) - v.root = container.NewBorder(top, v.summaryLabel, nil, nil, mainSplit) + bottom := container.NewVBox(v.runningStrip, v.summaryLabel) + v.root = container.NewBorder(top, bottom, nil, nil, mainSplit) +} + +// refreshRunningRows rebuilds runningRowsBox's Label children from +// runningRows. Called after every change to runningRows (Run/Stop). +func (v *EditorView) refreshRunningRows() { + labels := make([]fyne.CanvasObject, len(v.runningRows)) + for i, d := range v.runningRows { + labels[i] = widget.NewLabel(runningRowText(d)) + } + v.runningRowsBox.Objects = labels + v.runningRowsBox.Refresh() +} + +// runningRowText renders one running-devices strip entry. +func runningRowText(d simrun.RunningDevice) string { + return fmt.Sprintf("%d %s — %s:%d", d.ID, d.Name, d.Addr, d.Port) } // ---- device list ---- @@ -1095,12 +1191,34 @@ func (v *EditorView) revalidate() { if err != nil { v.summaryLabel.SetText(err.Error()) v.saveBtn.Disable() + v.valid = false } else { v.summaryLabel.SetText("valid") v.saveBtn.Enable() + v.valid = true } v.refreshFieldHints() v.refreshTitle() + v.updateRunButtons() +} + +// updateRunButtons enables/disables Run and Stop from the current running +// state and document validity: Stop is enabled only while a simulation is +// live; Run is enabled only while none is live and the document validates +// (mirroring saveBtn's gating — a document simrun would reject is never +// offered as runnable). +func (v *EditorView) updateRunButtons() { + if v.running != nil { + v.runBtn.Disable() + v.stopBtn.Enable() + return + } + v.stopBtn.Disable() + if v.valid { + v.runBtn.Enable() + } else { + v.runBtn.Disable() + } } // refreshFieldHints applies the current fieldErrors to whichever @@ -1144,21 +1262,9 @@ func (v *EditorView) onNew() { v.newDocument() } -// newDocument replaces the held document with a fresh scenariodoc.New() -// and refreshes every view that depends on it. +// newDocument replaces the held document with a fresh scenariodoc.New(). func (v *EditorView) newDocument() { - v.doc = scenariodoc.New() - v.selectedDevice = -1 - v.selectedObject = -1 - v.deviceList.Refresh() - v.refreshNetworkForm() - if len(v.doc.Scenario().Devices) > 0 { - v.selectDevice(0) - } else { - v.rebuildDeviceForm() - v.rebuildObjectForm() - } - v.revalidate() + v.replaceDocument(scenariodoc.New()) } // openPath loads the scenario at path, replacing the held document on @@ -1169,6 +1275,15 @@ func (v *EditorView) openPath(path string) error { if err != nil { return err } + v.replaceDocument(doc) + return nil +} + +// replaceDocument swaps in doc as the held document and refreshes every +// view that depends on it: the device list, the network form, the +// device/object detail forms (re-selecting the first device if any), and +// validation. Shared by New, Open, and Load example scenario. +func (v *EditorView) replaceDocument(doc *scenariodoc.Document) { v.doc = doc v.selectedDevice = -1 v.selectedObject = -1 @@ -1181,7 +1296,183 @@ func (v *EditorView) openPath(path string) error { v.rebuildObjectForm() } v.revalidate() - return nil +} + +// onLoadExample loads the bundled example scenario (assets.QuickstartScenario) +// into the editor, replacing the current document — the old one-click +// Quickstart demo becomes: Simulator -> Load example scenario -> Run. If +// the current document has unsaved edits, it confirms before discarding +// them. +func (v *EditorView) onLoadExample() { + if v.doc.Dirty() { + if win := currentWindow(); win != nil { + dialog.ShowConfirm( + "Unsaved changes", + "Loading the example scenario will discard unsaved changes. Continue?", + func(ok bool) { + if ok { + v.loadExampleScenario() + } + }, + win, + ) + return + } + } + v.loadExampleScenario() +} + +// loadExampleScenario decodes assets.QuickstartScenario and replaces the +// held document with it. +func (v *EditorView) loadExampleScenario() { + doc, err := scenariodoc.LoadBytes(assets.QuickstartScenario, "yaml") + if err != nil { + v.shell.SetStatus("example scenario invalid: " + err.Error()) + return + } + v.replaceDocument(doc) +} + +// ---- run / stop ---- + +// onRun validates the current document, then serializes it into a +// *simulator.Scenario (the document's own live Scenario() accessor) and +// starts it via StartRunner (simrun.Start in production, on a loopback +// UDP responder per device). A scenario simrun can't run at all — anything +// other than a loopback multi-port/single-device scenario — is reported in +// plain language rather than the raw error text. +func (v *EditorView) onRun() { + if err := v.doc.Validate(); err != nil { + v.shell.SetStatus("scenario is invalid: " + err.Error()) + return + } + + v.runBtn.Disable() + + sc := v.doc.Scenario() + + done := v.startDone + go func() { + if done != nil { + defer close(done) + } + + r, err := v.StartRunner(context.Background(), sc) + if err != nil { + fyne.Do(func() { v.updateRunButtons() }) + if errors.Is(err, simrun.ErrUnsupportedScenario) { + v.shell.SetStatus("Simulations run privately on this computer. Set the network mode to multi-port with loopback addresses (or use the example scenario).") + } else { + v.shell.SetStatus("simulation start failed: " + err.Error()) + } + return + } + + rows := r.Devices() + for _, d := range rows { + v.devices.Upsert(store.DeviceRow{ + Key: store.DeviceKey{Instance: d.ID, IP: d.Addr}, + Port: d.Port, + Name: d.Name, + Source: "simulated", + }) + } + + v.errWatchStop = make(chan struct{}) + go v.watchErrors(r, v.errWatchStop) + + fyne.Do(func() { + v.running = r + v.runningRows = rows + v.refreshRunningRows() + v.runningStrip.Show() + // v.root (not just runningStrip) needs a Refresh: root's + // Border layout sized its bottom region from runningStrip's + // MinSize while it was hidden (zero); only re-running that + // layout, not merely repainting runningStrip, makes room for + // it now that it is visible. + v.root.Refresh() + v.updateRunButtons() + }) + + status := fmt.Sprintf("Simulation running — %d devices (ports %s)", len(rows), portsText(rows)) + if v.PortHint != nil { + if hint := v.PortHint(portsOf(rows)); hint != "" { + status += " " + hint + } + } + v.shell.SetStatus(status) + }() +} + +// watchErrors forwards fatal runner errors to the status bar until stop is +// closed. +func (v *EditorView) watchErrors(r simRunner, stop <-chan struct{}) { + for { + select { + case err := <-r.Err(): + v.shell.SetStatus("simulation error: " + err.Error()) + case <-stop: + return + } + } +} + +// onStop shuts the running simulation down, removes its devices from the +// DeviceStore, and resets the Run/Stop buttons. +func (v *EditorView) onStop() { + v.stopBtn.Disable() + + r := v.running + if r == nil { + fyne.Do(func() { v.updateRunButtons() }) + return + } + + done := v.stopDone + go func() { + if done != nil { + defer close(done) + } + + if v.errWatchStop != nil { + close(v.errWatchStop) + v.errWatchStop = nil + } + r.Stop() + + for _, d := range v.runningRows { + v.devices.Remove(store.DeviceKey{Instance: d.ID, IP: d.Addr}) + } + fyne.Do(func() { + v.running = nil + v.runningRows = nil + v.refreshRunningRows() + v.runningStrip.Hide() + v.root.Refresh() + v.updateRunButtons() + }) + v.shell.SetStatus("Simulation stopped") + }() +} + +// portsText renders rows' ports as a comma-separated list for the +// "Simulation running" status text. +func portsText(rows []simrun.RunningDevice) string { + parts := make([]string, len(rows)) + for i, r := range rows { + parts[i] = strconv.FormatUint(uint64(r.Port), 10) + } + return strings.Join(parts, ", ") +} + +// portsOf extracts rows' ports, in order, for PortHint. +func portsOf(rows []simrun.RunningDevice) []uint16 { + ports := make([]uint16, len(rows)) + for i, r := range rows { + ports[i] = r.Port + } + return ports } // save writes the document to its current path. Returns scenariodoc.ErrNoPath diff --git a/gui/internal/ui/editor_test.go b/gui/internal/ui/editor_test.go index 1d8cedf..e169f14 100644 --- a/gui/internal/ui/editor_test.go +++ b/gui/internal/ui/editor_test.go @@ -1,33 +1,43 @@ package ui import ( + "bytes" + "context" "path/filepath" "reflect" "strings" "testing" + "time" + "fyne.io/fyne/v2" "fyne.io/fyne/v2/test" + "fyne.io/fyne/v2/widget" + "github.com/zyra/gobac/gui/assets" "github.com/zyra/gobac/gui/internal/scenariodoc" + "github.com/zyra/gobac/gui/internal/simrun" + "github.com/zyra/gobac/gui/internal/store" + "github.com/zyra/gobac/v2/simulator" ) // newEditorTestView builds an EditorView inside a headless test app/window // (dialog.NewFileOpen/NewFileSave need a window to attach to, even though // these tests never open them), returning the concrete type for -// same-package field access. -func newEditorTestView(t *testing.T) *EditorView { +// same-package field access and the DeviceStore it was wired to. +func newEditorTestView(t *testing.T) (*EditorView, *store.DeviceStore) { t.Helper() a := test.NewApp() w := a.NewWindow("test") t.Cleanup(w.Close) shell := NewAppShell(a, w) - obj := NewEditorView(shell) + devices := store.NewDeviceStore() + obj := NewEditorView(devices, shell) view, ok := obj.(*EditorView) if !ok { t.Fatalf("NewEditorView returned %T, want *EditorView", obj) } - return view + return view, devices } // TestNewDocumentHasOneDeviceAndValidates covers the brief's "New -> Add @@ -35,7 +45,7 @@ func newEditorTestView(t *testing.T) *EditorView { // seeds one device, so a freshly constructed view's device list already // shows exactly that device and the document already validates. func TestNewDocumentHasOneDeviceAndValidates(t *testing.T) { - view := newEditorTestView(t) + view, _ := newEditorTestView(t) if got, want := view.deviceListLength(), 1; got != want { t.Fatalf("deviceListLength() = %d, want %d", got, want) @@ -52,7 +62,7 @@ func TestNewDocumentHasOneDeviceAndValidates(t *testing.T) { // button, Present Value entry, and Writable check, then asserts the // resulting simulator.ObjectSpec fields directly. func TestAddAnalogValueObjectSetsExactFields(t *testing.T) { - view := newEditorTestView(t) + view, _ := newEditorTestView(t) view.objTypeSelect.SetSelected("analog-value") view.addObjectBtn.OnTapped() @@ -86,7 +96,7 @@ func TestAddAnalogValueObjectSetsExactFields(t *testing.T) { // Commandable -> Writable auto-check/disable behavior and the initial // priority 6 (reserved) / 8 (valid) field-error + Save-button transitions. func TestCommandableForcesWritableAndInitialPriorityValidates(t *testing.T) { - view := newEditorTestView(t) + view, _ := newEditorTestView(t) view.objTypeSelect.SetSelected("analog-value") view.addObjectBtn.OnTapped() view.objPresentValueEntry.SetText("21.5") @@ -130,7 +140,7 @@ func TestCommandableForcesWritableAndInitialPriorityValidates(t *testing.T) { // Address-required field error and its clearing on switching back to // single-device. func TestMultiIPModeRequiresAddressOnEmptyDevice(t *testing.T) { - view := newEditorTestView(t) + view, _ := newEditorTestView(t) view.modeSelect.SetSelected("multi-ip") @@ -160,7 +170,7 @@ func TestMultiIPModeRequiresAddressOnEmptyDevice(t *testing.T) { // (simulator.applyScenarioDefaults) that a never-loaded document never // had explicitly set. func TestSaveAsRoundTripsAndClearsDirty(t *testing.T) { - view := newEditorTestView(t) + view, _ := newEditorTestView(t) if err := view.openPath("testdata/scenario.yaml"); err != nil { t.Fatalf("openPath(testdata/scenario.yaml): %v", err) } @@ -212,3 +222,260 @@ func TestClassifyObjectType(t *testing.T) { } } } + +// ---- Run / Stop (task U3, migrated from the deleted quickstart_test.go) ---- + +// fakeSimRunner is a minimal simRunner fake: Devices returns a fixed set, +// Stop just records that it was called, and Err never fires. +type fakeSimRunner struct { + devices []simrun.RunningDevice + stopped bool + errCh chan error +} + +func newFakeSimRunner() *fakeSimRunner { + return &fakeSimRunner{ + devices: []simrun.RunningDevice{ + {ID: 1001, Name: "Boiler", Addr: "127.0.0.2", Port: 47901}, + {ID: 1002, Name: "AHU", Addr: "127.0.0.2", Port: 47902}, + }, + errCh: make(chan error), + } +} + +func (f *fakeSimRunner) Devices() []simrun.RunningDevice { return f.devices } +func (f *fakeSimRunner) Stop() { f.stopped = true } +func (f *fakeSimRunner) Err() <-chan error { return f.errCh } + +// awaitEditor blocks until done is closed (signaling the view's Run/Stop +// background goroutine has fully finished) or fails the test after a +// timeout, mirroring awaitSweep/awaitQuickstart's synchronization pattern. +func awaitEditor(t *testing.T, done chan struct{}) { + t.Helper() + select { + case <-done: + case <-time.After(30 * time.Second): + t.Fatal("run/stop operation did not complete within timeout") + } +} + +// lookupEditorRow finds the store row for (instance, ip), if any. +func lookupEditorRow(devices *store.DeviceStore, instance uint32, ip string) (store.DeviceRow, bool) { + for _, row := range devices.Snapshot() { + if row.Key.Instance == instance && row.Key.IP == ip { + return row, true + } + } + return store.DeviceRow{}, false +} + +// TestRunPopulatesDeviceStoreAndRunningStrip taps the rendered Run button +// and asserts DeviceStore rows appear with Source == "simulated" and Name +// set, the running-devices strip becomes visible, and the Stop button is +// enabled while Run stays disabled. +func TestRunPopulatesDeviceStoreAndRunningStrip(t *testing.T) { + view, devices := newEditorTestView(t) + fake := newFakeSimRunner() + view.StartRunner = func(ctx context.Context, sc *simulator.Scenario) (simRunner, error) { + return fake, nil + } + view.startDone = make(chan struct{}) + + test.Tap(view.runBtn) + awaitEditor(t, view.startDone) + + if got, want := devices.Len(), 2; got != want { + t.Fatalf("devices.Len() = %d, want %d", got, want) + } + for _, d := range fake.devices { + row, ok := lookupEditorRow(devices, d.ID, d.Addr) + if !ok { + t.Fatalf("store missing row for device %d/%s", d.ID, d.Addr) + } + if row.Source != "simulated" { + t.Errorf("device %d Source = %q, want %q", d.ID, row.Source, "simulated") + } + if row.Name != d.Name { + t.Errorf("device %d Name = %q, want %q", d.ID, row.Name, d.Name) + } + if row.Port != d.Port { + t.Errorf("device %d Port = %d, want %d", d.ID, row.Port, d.Port) + } + } + + if view.stopBtn.Disabled() { + t.Error("stop button should be enabled after a successful run") + } + if !view.runBtn.Disabled() { + t.Error("run button should stay disabled while the simulation is running") + } + if !view.runningStrip.Visible() { + t.Error("running-devices strip should be visible while a simulation is running") + } + if got, want := len(view.runningRowsBox.Objects), 2; got != want { + t.Errorf("len(runningRowsBox.Objects) = %d, want %d", got, want) + } + for i, want := range []string{ + "1001 Boiler — 127.0.0.2:47901", + "1002 AHU — 127.0.0.2:47902", + } { + lbl, ok := view.runningRowsBox.Objects[i].(*widget.Label) + if !ok { + t.Fatalf("runningRowsBox.Objects[%d] = %T, want *widget.Label", i, view.runningRowsBox.Objects[i]) + } + if lbl.Text != want { + t.Errorf("runningRowsBox.Objects[%d].Text = %q, want %q", i, lbl.Text, want) + } + } + + got := view.shell.Status.Text + if !strings.HasPrefix(got, "Simulation running — 2 devices (ports ") { + t.Errorf("status = %q, want prefix %q", got, "Simulation running — 2 devices (ports ") + } +} + +// TestRunAppendsPortHintToStatus covers the boot.go-wired PortHint seam: a +// non-empty return value is appended to the running-status text, and the +// callback receives every running device's port. +func TestRunAppendsPortHintToStatus(t *testing.T) { + view, _ := newEditorTestView(t) + fake := newFakeSimRunner() + view.StartRunner = func(ctx context.Context, sc *simulator.Scenario) (simRunner, error) { + return fake, nil + } + var gotPorts []uint16 + view.PortHint = func(ports []uint16) string { + gotPorts = ports + return "Tip: set Settings → Port to 47901 to interact with these devices." + } + view.startDone = make(chan struct{}) + + test.Tap(view.runBtn) + awaitEditor(t, view.startDone) + + want := "Simulation running — 2 devices (ports 47901, 47902) Tip: set Settings → Port to 47901 to interact with these devices." + if got := view.shell.Status.Text; got != want { + t.Errorf("status = %q, want %q", got, want) + } + wantPorts := []uint16{47901, 47902} + if len(gotPorts) != len(wantPorts) { + t.Fatalf("PortHint received %d ports, want %d", len(gotPorts), len(wantPorts)) + } + for i, p := range wantPorts { + if gotPorts[i] != p { + t.Errorf("PortHint ports[%d] = %d, want %d", i, gotPorts[i], p) + } + } +} + +// TestStopRemovesInjectedRowsAndHidesStrip taps Run then Stop and asserts +// the injected rows are removed, the runner's Stop was called, the strip +// hides again, and the buttons reset. +func TestStopRemovesInjectedRowsAndHidesStrip(t *testing.T) { + view, devices := newEditorTestView(t) + fake := newFakeSimRunner() + view.StartRunner = func(ctx context.Context, sc *simulator.Scenario) (simRunner, error) { + return fake, nil + } + view.startDone = make(chan struct{}) + + test.Tap(view.runBtn) + awaitEditor(t, view.startDone) + + if got, want := devices.Len(), 2; got != want { + t.Fatalf("precondition: devices.Len() = %d, want %d", got, want) + } + + view.stopDone = make(chan struct{}) + test.Tap(view.stopBtn) + awaitEditor(t, view.stopDone) + + if !fake.stopped { + t.Error("expected the runner's Stop to have been called") + } + if got, want := devices.Len(), 0; got != want { + t.Errorf("devices.Len() after stop = %d, want %d", got, want) + } + if view.runningStrip.Visible() { + t.Error("running-devices strip should hide after Stop") + } + if got, want := len(view.runningRowsBox.Objects), 0; got != want { + t.Errorf("len(runningRowsBox.Objects) after stop = %d, want %d", got, want) + } + if view.runBtn.Disabled() { + t.Error("run button should be re-enabled after stop") + } + if !view.stopBtn.Disabled() { + t.Error("stop button should be disabled after stop") + } + if got, want := view.shell.Status.Text, "Simulation stopped"; got != want { + t.Errorf("status = %q, want %q", got, want) + } +} + +// TestRunRejectsNonLoopbackScenarioWithPlainLanguageMessage covers the +// simrun.ErrUnsupportedScenario path: Run never starts the runner (no +// devices injected) and the status bar shows plain-language guidance +// rather than the raw error text. +func TestRunRejectsNonLoopbackScenarioWithPlainLanguageMessage(t *testing.T) { + view, devices := newEditorTestView(t) + view.StartRunner = func(ctx context.Context, sc *simulator.Scenario) (simRunner, error) { + return nil, simrun.ErrUnsupportedScenario + } + view.startDone = make(chan struct{}) + + test.Tap(view.runBtn) + awaitEditor(t, view.startDone) + + want := "Simulations run privately on this computer. Set the network mode to multi-port with loopback addresses (or use the example scenario)." + if got := view.shell.Status.Text; got != want { + t.Errorf("status = %q, want %q", got, want) + } + if view.running != nil { + t.Error("view.running should be nil after a rejected Run") + } + if devices.Len() != 0 { + t.Errorf("devices.Len() = %d, want 0 (no devices injected on rejection)", devices.Len()) + } + if view.runBtn.Disabled() { + t.Error("run button should be re-enabled after a rejected Run") + } + if !view.stopBtn.Disabled() { + t.Error("stop button should stay disabled after a rejected Run") + } +} + +// TestLoadExampleScenarioReplacesDeviceList taps "Load example scenario" on +// a fresh (non-dirty) document and asserts the editor's device list renders +// the bundled example's devices: the rendered capture changes and the +// device count matches the example scenario's device count exactly. +func TestLoadExampleScenarioReplacesDeviceList(t *testing.T) { + view, _ := newEditorTestView(t) + + a := test.NewApp() + w := a.NewWindow("capture") + defer w.Close() + w.SetContent(view) + w.Resize(fyne.NewSize(900, 600)) + + before := w.Canvas().Capture() + + test.Tap(view.loadExampleBtn) + + after := w.Canvas().Capture() + if imagesEqual(before, after) { + t.Fatal("canvas capture is unchanged after tapping Load example scenario") + } + + exampleScenario, err := simulator.DecodeScenario(bytes.NewReader(assets.QuickstartScenario), "yaml") + if err != nil { + t.Fatalf("decode bundled example scenario: %v", err) + } + want := len(exampleScenario.Devices) + if got := view.deviceListLength(); got != want { + t.Errorf("deviceListLength() after Load example scenario = %d, want %d (example device count)", got, want) + } + if got, want := view.summaryLabel.Text, "valid"; got != want { + t.Errorf("summaryLabel.Text = %q, want %q", got, want) + } +} diff --git a/gui/internal/ui/quickstart.go b/gui/internal/ui/quickstart.go deleted file mode 100644 index 7853c6e..0000000 --- a/gui/internal/ui/quickstart.go +++ /dev/null @@ -1,220 +0,0 @@ -package ui - -import ( - "bytes" - "context" - "fmt" - - "fyne.io/fyne/v2" - "fyne.io/fyne/v2/container" - "fyne.io/fyne/v2/widget" - - "github.com/zyra/gobac/gui/assets" - "github.com/zyra/gobac/gui/internal/simrun" - "github.com/zyra/gobac/gui/internal/store" - "github.com/zyra/gobac/v2/simulator" -) - -// quickstartDescription is the Quickstart view's static explanatory text. -const quickstartDescription = "Run a bundled demo scenario in-process on loopback UDP — no external processes. Its devices appear in Discovery for browsing, reading, and writing over the real client path." - -// quickstartRunner is the subset of *simrun.Runner the Quickstart view -// depends on, so tests can substitute a fake instead of running real UDP -// sockets. -type quickstartRunner interface { - Devices() []simrun.RunningDevice - Stop() - Err() <-chan error -} - -var _ quickstartRunner = (*simrun.Runner)(nil) - -// QuickstartView is the Quickstart navigation entry (task G8): a one-click -// in-process simulator run whose devices are injected into the shared -// DeviceStore (Source "local-sim") so Discovery and the Object Browser can -// exercise them over real loopback UDP. -// -// QuickstartView is a proper widget (widget.BaseWidget + CreateRenderer) -// rather than an embedded *fyne.Container; see the identical note on -// DiscoveryView in discovery.go. -type QuickstartView struct { - widget.BaseWidget - - devices *store.DeviceStore - shell *AppShell - - startBtn *widget.Button - stopBtn *widget.Button - list *widget.List - - running quickstartRunner - rows []simrun.RunningDevice - - errWatchStop chan struct{} - - // StartRunner starts a decoded scenario. Exported so tests can replace - // it with a fake runner instead of exercising real UDP sockets. - // Defaults to wrapping simrun.Start. - StartRunner func(ctx context.Context, sc *simulator.Scenario) (quickstartRunner, error) - - // startDone/stopDone are test-only synchronization seams, mirroring - // DiscoveryView.sweepDone: if non-nil when the corresponding method is - // invoked, each is closed once that call's background goroutine - // finishes all of its work (including any fyne.Do UI update). - // Production code leaves them nil. - startDone chan struct{} - stopDone chan struct{} - - root *fyne.Container -} - -// NewQuickstartView builds the Quickstart view. -func NewQuickstartView(devices *store.DeviceStore, shell *AppShell) fyne.CanvasObject { - v := &QuickstartView{ - devices: devices, - shell: shell, - StartRunner: defaultStartRunner, - } - - description := widget.NewLabel(quickstartDescription) - description.Wrapping = fyne.TextWrapWord - - v.startBtn = widget.NewButton("Start local simulator", v.start) - v.stopBtn = widget.NewButton("Stop", v.stop) - v.stopBtn.Disable() - - toolbar := container.NewHBox(v.startBtn, v.stopBtn) - - v.list = widget.NewList( - func() int { return len(v.rows) }, - func() fyne.CanvasObject { return widget.NewLabel("") }, - func(id widget.ListItemID, obj fyne.CanvasObject) { - obj.(*widget.Label).SetText(v.rowText(id)) - }, - ) - - v.root = container.NewBorder( - container.NewVBox(description, toolbar), nil, nil, nil, v.list, - ) - v.ExtendBaseWidget(v) - - return v -} - -// CreateRenderer implements fyne.Widget. -func (v *QuickstartView) CreateRenderer() fyne.WidgetRenderer { - return widget.NewSimpleRenderer(v.root) -} - -// defaultStartRunner decodes sc and starts it via simrun.Start, adapting -// *simrun.Runner to the quickstartRunner interface. -func defaultStartRunner(ctx context.Context, sc *simulator.Scenario) (quickstartRunner, error) { - return simrun.Start(ctx, sc) -} - -// rowText renders the running-device list entry at id. -func (v *QuickstartView) rowText(id widget.ListItemID) string { - if id < 0 || id >= len(v.rows) { - return "" - } - d := v.rows[id] - return fmt.Sprintf("%d %s — %s:%d", d.ID, d.Name, d.Addr, d.Port) -} - -// start decodes the bundled quickstart scenario, starts it, and injects its -// devices into the shared DeviceStore as Source "local-sim" rows. -func (v *QuickstartView) start() { - v.startBtn.Disable() - - done := v.startDone - go func() { - if done != nil { - defer close(done) - } - - sc, err := simulator.DecodeScenario(bytes.NewReader(assets.QuickstartScenario), "yaml") - if err != nil { - fyne.Do(func() { v.startBtn.Enable() }) - v.shell.SetStatus("quickstart scenario invalid: " + err.Error()) - return - } - - r, err := v.StartRunner(context.Background(), sc) - if err != nil { - fyne.Do(func() { v.startBtn.Enable() }) - v.shell.SetStatus("quickstart start failed: " + err.Error()) - return - } - - rows := r.Devices() - for _, d := range rows { - v.devices.Upsert(store.DeviceRow{ - Key: store.DeviceKey{Instance: d.ID, IP: d.Addr}, - Port: d.Port, - Source: "local-sim", - }) - } - - v.errWatchStop = make(chan struct{}) - go v.watchErrors(r, v.errWatchStop) - - fyne.Do(func() { - v.running = r - v.rows = rows - v.list.Refresh() - v.startBtn.Disable() - v.stopBtn.Enable() - }) - v.shell.SetStatus(fmt.Sprintf("local simulator running (%d devices)", len(rows))) - }() -} - -// watchErrors forwards fatal runner errors to the status bar until stop is -// closed. -func (v *QuickstartView) watchErrors(r quickstartRunner, stop <-chan struct{}) { - for { - select { - case err := <-r.Err(): - v.shell.SetStatus("simulator error: " + err.Error()) - case <-stop: - return - } - } -} - -// stop shuts the running simulator down, removes its devices from the -// DeviceStore, and resets the buttons. -func (v *QuickstartView) stop() { - v.stopBtn.Disable() - - r := v.running - if r == nil { - fyne.Do(func() { v.startBtn.Enable() }) - return - } - - done := v.stopDone - go func() { - if done != nil { - defer close(done) - } - - if v.errWatchStop != nil { - close(v.errWatchStop) - v.errWatchStop = nil - } - r.Stop() - - for _, d := range v.rows { - v.devices.Remove(store.DeviceKey{Instance: d.ID, IP: d.Addr}) - } - fyne.Do(func() { - v.running = nil - v.rows = nil - v.list.Refresh() - v.startBtn.Enable() - v.stopBtn.Disable() - }) - v.shell.SetStatus("local simulator stopped") - }() -} diff --git a/gui/internal/ui/quickstart_test.go b/gui/internal/ui/quickstart_test.go deleted file mode 100644 index da4f2d3..0000000 --- a/gui/internal/ui/quickstart_test.go +++ /dev/null @@ -1,169 +0,0 @@ -package ui - -import ( - "context" - "testing" - "time" - - "fyne.io/fyne/v2/test" - "fyne.io/fyne/v2/widget" - - "github.com/zyra/gobac/gui/internal/simrun" - "github.com/zyra/gobac/gui/internal/store" - "github.com/zyra/gobac/v2/simulator" -) - -// fakeQuickstartRunner is a minimal quickstartRunner fake: Devices returns a -// fixed set, Stop just records that it was called, and Err never fires. -type fakeQuickstartRunner struct { - devices []simrun.RunningDevice - stopped bool - errCh chan error -} - -func newFakeQuickstartRunner() *fakeQuickstartRunner { - return &fakeQuickstartRunner{ - devices: []simrun.RunningDevice{ - {ID: 1001, Name: "Boiler", Addr: "127.0.0.2", Port: 47901}, - {ID: 1002, Name: "AHU", Addr: "127.0.0.2", Port: 47902}, - {ID: 1003, Name: "Lab Sensor", Addr: "127.0.0.2", Port: 47903}, - }, - errCh: make(chan error), - } -} - -func (f *fakeQuickstartRunner) Devices() []simrun.RunningDevice { return f.devices } -func (f *fakeQuickstartRunner) Stop() { f.stopped = true } -func (f *fakeQuickstartRunner) Err() <-chan error { return f.errCh } - -// newQuickstartTestView builds a QuickstartView wired to a fresh DeviceStore -// and a fake StartRunner, returning the concrete type for same-package field -// access. -func newQuickstartTestView(t *testing.T, fake *fakeQuickstartRunner, startErr error) (*QuickstartView, *store.DeviceStore) { - t.Helper() - a := test.NewApp() - w := a.NewWindow("test") - t.Cleanup(w.Close) - - shell := NewAppShell(a, w) - devices := store.NewDeviceStore() - - obj := NewQuickstartView(devices, shell) - view, ok := obj.(*QuickstartView) - if !ok { - t.Fatalf("NewQuickstartView returned %T, want *QuickstartView", obj) - } - view.StartRunner = func(ctx context.Context, sc *simulator.Scenario) (quickstartRunner, error) { - if startErr != nil { - return nil, startErr - } - return fake, nil - } - return view, devices -} - -func awaitQuickstart(t *testing.T, done chan struct{}) { - t.Helper() - select { - case <-done: - case <-time.After(30 * time.Second): - // Generous for slow CI runners (see awaitBrowser in - // browser_test.go); a passing wait returns immediately. - t.Fatal("quickstart operation did not complete within timeout") - } -} - -func listText(view *QuickstartView, id widget.ListItemID) string { - label := widget.NewLabel("") - view.list.UpdateItem(id, label) - return label.Text -} - -func TestStartPopulatesDeviceListAndStore(t *testing.T) { - fake := newFakeQuickstartRunner() - view, devices := newQuickstartTestView(t, fake, nil) - view.startDone = make(chan struct{}) - - test.Tap(view.startBtn) - awaitQuickstart(t, view.startDone) - - if got := view.list.Length(); got != 3 { - t.Fatalf("list length = %d, want 3", got) - } - want := []string{ - "1001 Boiler — 127.0.0.2:47901", - "1002 AHU — 127.0.0.2:47902", - "1003 Lab Sensor — 127.0.0.2:47903", - } - for i, wantText := range want { - if got := listText(view, widget.ListItemID(i)); got != wantText { - t.Errorf("row %d = %q, want %q", i, got, wantText) - } - } - - if got, want := devices.Len(), 3; got != want { - t.Fatalf("devices.Len() = %d, want %d", got, want) - } - for _, d := range fake.devices { - row, ok := lookupRow(devices, d.ID, d.Addr) - if !ok { - t.Fatalf("store missing row for device %d/%s", d.ID, d.Addr) - } - if row.Source != "local-sim" { - t.Errorf("device %d Source = %q, want %q", d.ID, row.Source, "local-sim") - } - if row.Port != d.Port { - t.Errorf("device %d Port = %d, want %d", d.ID, row.Port, d.Port) - } - } - - if view.stopBtn.Disabled() { - t.Error("stop button should be enabled after a successful start") - } - if !view.startBtn.Disabled() { - t.Error("start button should stay disabled while the simulator is running") - } -} - -func TestStopRemovesInjectedRowsAndResetsButtons(t *testing.T) { - fake := newFakeQuickstartRunner() - view, devices := newQuickstartTestView(t, fake, nil) - view.startDone = make(chan struct{}) - - test.Tap(view.startBtn) - awaitQuickstart(t, view.startDone) - - if got, want := devices.Len(), 3; got != want { - t.Fatalf("precondition: devices.Len() = %d, want %d", got, want) - } - - view.stopDone = make(chan struct{}) - test.Tap(view.stopBtn) - awaitQuickstart(t, view.stopDone) - - if !fake.stopped { - t.Error("expected the runner's Stop to have been called") - } - if got, want := devices.Len(), 0; got != want { - t.Errorf("devices.Len() after stop = %d, want %d", got, want) - } - if got := view.list.Length(); got != 0 { - t.Errorf("list length after stop = %d, want 0", got) - } - if view.startBtn.Disabled() { - t.Error("start button should be re-enabled after stop") - } - if !view.stopBtn.Disabled() { - t.Error("stop button should be disabled after stop") - } -} - -// lookupRow finds the store row for (instance, ip), if any. -func lookupRow(devices *store.DeviceStore, instance uint32, ip string) (store.DeviceRow, bool) { - for _, row := range devices.Snapshot() { - if row.Key.Instance == instance && row.Key.IP == ip { - return row, true - } - } - return store.DeviceRow{}, false -} diff --git a/gui/internal/ui/shell.go b/gui/internal/ui/shell.go index 63a63fb..476af7f 100644 --- a/gui/internal/ui/shell.go +++ b/gui/internal/ui/shell.go @@ -8,12 +8,14 @@ import ( "fyne.io/fyne/v2/widget" ) -// navLabels are the left-navigation entries, in display order. -var navLabels = []string{"Discovery", "Object Browser", "Simulator Editor", "Quickstart"} +// navLabels are the left-navigation entries, in display order. Task U3 +// folded the former Quickstart view into the Simulator (Simulator Editor) +// view, leaving three entries. +var navLabels = []string{"Discovery", "Object Browser", "Simulator"} // viewLabels are the placeholder center-content texts, one per navLabels // entry at the same index. -var viewLabels = []string{"Discovery view", "Object browser", "Scenario editor", "Quickstart"} +var viewLabels = []string{"Discovery view", "Object browser", "Simulator"} // AppShell is the top-level content for the GoBAC Workstation main window: // a left navigation list, a center content stack that switches per diff --git a/gui/internal/ui/shell_test.go b/gui/internal/ui/shell_test.go index 76d5a6e..d21f0c0 100644 --- a/gui/internal/ui/shell_test.go +++ b/gui/internal/ui/shell_test.go @@ -10,14 +10,14 @@ import ( "fyne.io/fyne/v2/widget" ) -func TestNewAppShellNavigationHasFourLabeledItems(t *testing.T) { +func TestNewAppShellNavigationHasThreeLabeledItems(t *testing.T) { a := test.NewApp() w := a.NewWindow("test") defer w.Close() shell := NewAppShell(a, w) - want := []string{"Discovery", "Object Browser", "Simulator Editor", "Quickstart"} + want := []string{"Discovery", "Object Browser", "Simulator"} if got := len(navLabels); got != len(want) { t.Fatalf("len(navLabels) = %d, want %d", got, len(want)) @@ -58,7 +58,7 @@ func TestSelectingNavIndexSwitchesVisibleContent(t *testing.T) { t.Fatal("canvas capture is unchanged after selecting a different nav row") } - if got, want := visibleLabelText(t, shell.Content), "Scenario editor"; got != want { + if got, want := visibleLabelText(t, shell.Content), "Simulator"; got != want { t.Errorf("visible content = %q, want %q", got, want) } From faa47364cf04e89ac0d5a8a3eae7c10e862a27b6 Mon Sep 17 00:00:00 2001 From: Ibby Hadeed Date: Sat, 18 Jul 2026 20:38:19 +0400 Subject: [PATCH 04/10] Add rendered regression test for running-devices strip visibility Mounts the editor view into a real window/canvas and asserts the strip actually occupies rendered space and paints content once a simulation starts, catching the exact clipped/zero-size failure mode that v.root.Refresh() in onRun/onStop guards against. --- gui/internal/ui/editor_test.go | 64 ++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/gui/internal/ui/editor_test.go b/gui/internal/ui/editor_test.go index e169f14..eca9115 100644 --- a/gui/internal/ui/editor_test.go +++ b/gui/internal/ui/editor_test.go @@ -3,6 +3,7 @@ package ui import ( "bytes" "context" + "image" "path/filepath" "reflect" "strings" @@ -334,6 +335,55 @@ func TestRunPopulatesDeviceStoreAndRunningStrip(t *testing.T) { } } +// TestRunMakesRunningStripVisibleOnCanvas mounts the editor view into a real +// window/canvas (unlike TestRunPopulatesDeviceStoreAndRunningStrip, which +// only inspects Visible()/Objects without ever rendering) and asserts the +// running-devices strip actually occupies rendered space and paints content +// once a simulation starts. This is the regression test for the +// v.root.Refresh() fix in onRun/onStop: runningStrip.Show() alone leaves the +// Border layout's bottom region sized from when the strip was hidden +// (zero), so without re-running that layout the strip would report +// Visible() == true while remaining invisibly clipped to zero size — a +// capture-only test catches that; a Visible()/Objects-only test does not. +func TestRunMakesRunningStripVisibleOnCanvas(t *testing.T) { + view, _ := newEditorTestView(t) + fake := newFakeSimRunner() + view.StartRunner = func(ctx context.Context, sc *simulator.Scenario) (simRunner, error) { + return fake, nil + } + view.startDone = make(chan struct{}) + + a := test.NewApp() + w := a.NewWindow("capture") + defer w.Close() + w.SetContent(view) + w.Resize(fyne.NewSize(900, 700)) + + before := w.Canvas().Capture() + + test.Tap(view.runBtn) + awaitEditor(t, view.startDone) + + after := w.Canvas().Capture() + if imagesEqual(before, after) { + t.Fatal("canvas capture is unchanged after Run; running-devices strip never became visible") + } + + stripSize := view.runningStrip.Size() + if stripSize.Width <= 0 || stripSize.Height <= 0 { + t.Fatalf("runningStrip.Size() = %v, want > 0 in both dimensions (strip must occupy real layout space once visible, not just report Visible() == true)", stripSize) + } + + stripPos := view.runningStrip.Position() + region := image.Rect( + int(stripPos.X), int(stripPos.Y), + int(stripPos.X+stripSize.Width), int(stripPos.Y+stripSize.Height), + ) + if got := distinctColorsInRegion(after, region); got <= 1 { + t.Fatalf("running-devices strip region has %d distinct color(s), want > 1 (region should render device rows, not blank/clipped content)", got) + } +} + // TestRunAppendsPortHintToStatus covers the boot.go-wired PortHint seam: a // non-empty return value is appended to the running-status text, and the // callback receives every running device's port. @@ -479,3 +529,17 @@ func TestLoadExampleScenarioReplacesDeviceList(t *testing.T) { t.Errorf("summaryLabel.Text = %q, want %q", got, want) } } + +// distinctColorsInRegion returns the number of distinct pixel colors within +// the intersection of img's bounds and region. +func distinctColorsInRegion(img image.Image, region image.Rectangle) int { + b := img.Bounds().Intersect(region) + seen := make(map[[4]uint32]struct{}) + for y := b.Min.Y; y < b.Max.Y; y++ { + for x := b.Min.X; x < b.Max.X; x++ { + r, g, bch, aCh := img.At(x, y).RGBA() + seen[[4]uint32{r, g, bch, aCh}] = struct{}{} + } + } + return len(seen) +} From 283b39534b9f8492d96014e714482961412e8f59 Mon Sep 17 00:00:00 2001 From: Ibby Hadeed Date: Sat, 18 Jul 2026 21:05:32 +0400 Subject: [PATCH 05/10] Add welcoming Home view and rework navigation to plain language Home greets first-run users with two big actions (Simulate a network, Discover my network) plus a Settings shortcut instead of a blank pane. Nav trims to Home / Network Explorer / Simulator; Object Browser is now a drill-down of Network Explorer with a Back-to-results button rather than its own top-level row. Discovery gets plain-language copy (Scan network, an empty-state placeholder) and a launch failure now reports "Not connected yet" instead of a raw error. --- gui/internal/boot/boot.go | 136 ++++++++++++++++++++----- gui/internal/boot/boot_test.go | 164 ++++++++++++++++++++++++++++-- gui/internal/ui/discovery.go | 39 +++++-- gui/internal/ui/discovery_test.go | 49 +++++++++ gui/internal/ui/home.go | 54 ++++++++++ gui/internal/ui/home_test.go | 88 ++++++++++++++++ gui/internal/ui/shell.go | 23 +++-- gui/internal/ui/shell_test.go | 28 ++++- 8 files changed, 528 insertions(+), 53 deletions(-) create mode 100644 gui/internal/ui/home.go create mode 100644 gui/internal/ui/home_test.go diff --git a/gui/internal/boot/boot.go b/gui/internal/boot/boot.go index 068818a..c01c246 100644 --- a/gui/internal/boot/boot.go +++ b/gui/internal/boot/boot.go @@ -9,6 +9,9 @@ import ( "net" "fyne.io/fyne/v2" + "fyne.io/fyne/v2/container" + "fyne.io/fyne/v2/theme" + "fyne.io/fyne/v2/widget" "github.com/zyra/gobac/gui/internal/netpick" "github.com/zyra/gobac/gui/internal/session" @@ -16,22 +19,23 @@ import ( "github.com/zyra/gobac/gui/internal/ui" ) -// discoveryNavIndex, browserNavIndex, and editorNavIndex are the AppShell -// nav indices of the Discovery, Object Browser, and Simulator views (see -// navLabels in internal/ui/shell.go). Task U3 folded the former Quickstart -// view into the Simulator (editor) view, so there is no separate nav index -// for it anymore. +// homeNavIndex, explorerNavIndex, and simulatorNavIndex are the AppShell nav +// indices of the Home, Network Explorer, and Simulator views (see navLabels +// in internal/ui/shell.go). Task U4 added Home and renamed Discovery to +// Network Explorer; the former Object Browser nav row is gone — viewing a +// device's objects is now a drill-down inside the Network Explorer slot +// (see newExplorerPane), not a separate top-level destination. const ( - discoveryNavIndex = 0 - browserNavIndex = 1 - editorNavIndex = 2 + homeNavIndex = 0 + explorerNavIndex = 1 + simulatorNavIndex = 2 ) // Compose builds the application shell and wires it to sess: window sizing // and main menu, persisted settings, session start (non-fatal on failure), -// close intercept, stores, every view, the Discovery-to-Browser selection -// handoff, and window.SetContent. It returns the composed shell so callers -// (and tests) can drive it further. +// close intercept, stores, every view, Home's primary actions, the Network +// Explorer's device-row-to-Object-Browser drill-down, and window.SetContent. +// It returns the composed shell so callers (and tests) can drive it further. func Compose(a fyne.App, w fyne.Window, sess session.Session) *ui.AppShell { w.Resize(fyne.NewSize(1100, 700)) @@ -43,10 +47,11 @@ func Compose(a fyne.App, w fyne.Window, sess session.Session) *ui.AppShell { } w.SetMainMenu(ui.NewMainMenu(a, w, restart)) - // Startup goes through the same startSession helper Settings' Restart - // callback uses, so first launch and a live network change report - // identically in the status bar. - startSession(sess, shell, ui.LoadSettings(a)) + // Startup uses startLaunch rather than the startSession helper Settings' + // Restart callback uses: a failed first launch must never greet the user + // with a raw error (see the UX vision), so it reports gentler wording. A + // successful launch and a successful restart still read identically. + startLaunch(sess, shell, ui.LoadSettings(a)) w.SetCloseIntercept(func() { _ = session.Shutdown(sess) @@ -57,38 +62,113 @@ func Compose(a fyne.App, w fyne.Window, sess session.Session) *ui.AppShell { objects := store.NewObjectCache() discovery := ui.NewDiscoveryView(sess, devices, shell) - shell.SetView(discoveryNavIndex, discovery) - browser := ui.NewBrowserView(sess, objects, shell) - shell.SetView(browserNavIndex, browser) + + explorer := newExplorerPane(discovery, browser) + shell.SetView(explorerNavIndex, explorer.content) editor := ui.NewEditorView(devices, shell) - shell.SetView(editorNavIndex, editor) + shell.SetView(simulatorNavIndex, editor) if editorView, ok := editor.(*ui.EditorView); ok { editorView.PortHint = func(ports []uint16) string { return sessionPortHint(a, ports) } } - if discoveryView, ok := discovery.(*ui.DiscoveryView); ok { - if browserView, ok := browser.(*ui.BrowserView); ok { - discoveryView.OnSelect = func(row store.DeviceRow) { - browserView.LoadDevice(row) - shell.Nav.Select(browserNavIndex) - } + discoveryView, _ := discovery.(*ui.DiscoveryView) + browserView, _ := browser.(*ui.BrowserView) + if discoveryView != nil && browserView != nil { + discoveryView.OnSelect = func(row store.DeviceRow) { + browserView.LoadDevice(row) + explorer.showDetail() } } + home := ui.NewHomeView( + func() { shell.Select(simulatorNavIndex) }, + func() { + explorer.showList() + shell.Select(explorerNavIndex) + if discoveryView != nil { + discoveryView.Sweep() + } + }, + func() { ui.NewSettingsDialog(a, w, restart).Show() }, + ) + shell.SetView(homeNavIndex, home) + shell.Select(homeNavIndex) + w.SetContent(shell) return shell } +// explorerPane composes the Network Explorer nav slot (task U4): the device +// list (Discovery) is the default view; selecting a device row swaps in the +// Object Browser detail with a "Back to results" button, so browsing a +// device's objects reads as a drill-down of Network Explorer rather than a +// separate top-level destination — only Home, Network Explorer, and +// Simulator are real nav rows in the rework. +type explorerPane struct { + content *fyne.Container + list fyne.CanvasObject + detail *fyne.Container +} + +// newExplorerPane wraps list (the Discovery view) and detail (the Browser +// view) into a single content object for shell.SetView(explorerNavIndex, ...). +func newExplorerPane(list, detail fyne.CanvasObject) *explorerPane { + p := &explorerPane{list: list} + + back := widget.NewButtonWithIcon("Back to results", theme.NavigateBackIcon(), p.showList) + p.detail = container.NewBorder(back, nil, nil, nil, detail) + p.detail.Hide() + + p.content = container.NewStack(list, p.detail) + return p +} + +// showDetail swaps the pane to the Object Browser detail (a device row was +// selected in the device list). +func (p *explorerPane) showDetail() { + p.list.Hide() + p.detail.Show() +} + +// showList swaps the pane back to the device list — via the "Back to +// results" button, or programmatically before a fresh scan (Home's +// "Discover my network" always starts from the list, never mid-drill-down). +func (p *explorerPane) showList() { + p.detail.Hide() + p.list.Show() +} + +// startLaunch starts sess using s for the app's very first launch and +// reports the outcome in shell's status bar. Success reads identically to a +// Settings restart ("Connected on