commit c541eeee0ae03b47c51dd649fa8af76830c9bc92
| author | 斟酌 鵬兄 <tgckpg@gmail.com> |
|---|
| date | 2026-10-15T11:32:40Z |
|---|
| subject | Fixed a bug that disconnect could report success after removing a failed preparation |
|---|
commit c541eeee0ae03b47c51dd649fa8af76830c9bc92
Author: 斟酌 鵬兄 <tgckpg@gmail.com>
Date: 2026-10-15T11:32:40Z
Fixed a bug that disconnect could report success after removing a failed preparation
---
build.env | 2 +-
drivers/sesame/README.md | 12 ++++++++
drivers/sesame/bluez.go | 44 +++++++++++++++++++++++++--
drivers/sesame/bluez_test.go | 59 +++++++++++++++++++++++++++++--------
drivers/sesame/connections.go | 2 +-
ha-app/CHANGELOG.md | 4 +++
ha-app/config.yaml | 2 +-
internal/app/physical.go | 13 ++++----
internal/app/physical_bluez_test.go | 59 +++++++++++++++++++++++++++++++++++++
9 files changed, 174 insertions(+), 23 deletions(-)
diff --git a/build.env b/build.env
index 51411bd..0f0870d 100644
--- a/build.env
+++ b/build.env
@@ -1,4 +1,4 @@
IMAGE_NAME=ghcr.io/tgckpg/homekey-go
-IMAGE_TAG=0.0.16
+IMAGE_TAG=0.0.17
APT_PROXY=
diff --git a/drivers/sesame/README.md b/drivers/sesame/README.md
index ecd23ce..6da1cb0 100644
--- a/drivers/sesame/README.md
+++ b/drivers/sesame/README.md
@@ -108,3 +108,15 @@ the old 8-second GATT budget expired. GATT now allows 45 seconds within a
setup does not consume the separate initial-token and registration budgets.
Login and command timeouts are unchanged. The private-bus regression test also
simulates discovery lasting longer than eight seconds.
+
+Existing BLE link recovery: the manager continues to reuse its live ready or
+registered session. If a new dial finds this lock already connected in BlueZ,
+there is no retained token/cipher for that link, so it disconnects only that
+UUID on the selected adapter, waits for `Connected=false`, and scans/connects
+again for a fresh initial token. A connection completing during discovery is
+recovered through the bounded setup retry path too. This does not reset the
+lock or replay registration/motor commands. Manual Disconnect always checks the
+actual BlueZ device after retiring a pending preparation, including a failed
+preparation that never acquired the link. A regression covers this
+disconnect-action cleanup, and private-bus transport tests cover delayed disconnect completion and
+connections appearing during a scan.
diff --git a/drivers/sesame/bluez.go b/drivers/sesame/bluez.go
index 0afdf8d..b719bc6 100644
--- a/drivers/sesame/bluez.go
+++ b/drivers/sesame/bluez.go
@@ -238,6 +238,30 @@ func chooseTarget(o objects, t Target, seen map[dbus.ObjectPath]uint64) (dbus.Ob
return selected, found
}
+// disconnectDevice waits for BlueZ's link state, not just acceptance of the
+// Disconnect method. A timed-out setup can otherwise leave Connected=true and
+// make every later preparation reject the device before it can subscribe.
+func disconnectDevice(ctx context.Context, c *dbus.Conn, path dbus.ObjectPath) error {
+ cleanup, cancel := context.WithTimeout(ctx, 5*time.Second)
+ defer cancel()
+ callError := c.Object("org.bluez", path).CallWithContext(cleanup, "org.bluez.Device1.Disconnect", 0).Err
+ // A concurrent disconnect can make the method fail even though the desired
+ // state has been reached. Treat the observed link state as authoritative.
+ for {
+ o, e := getObjects(cleanup, c)
+ if e != nil {
+ return fmt.Errorf("confirm Sesame disconnect: %w", errors.Join(callError, e))
+ }
+ connected, _ := o[path]["org.bluez.Device1"]["Connected"].Value().(bool)
+ if !connected {
+ return nil
+ }
+ if e := pause(cleanup, 50*time.Millisecond); e != nil {
+ return fmt.Errorf("wait for Sesame link to disconnect: %w", errors.Join(callError, e))
+ }
+ }
+}
+
// StartDiscovery is reference counted. These signal matches and the scan belong
// only to this D-Bus client; do not remove BlueZ devices or reset the adapter.
func findTarget(ctx context.Context, c *dbus.Conn, t Target) (dbus.ObjectPath, Target, error) {
@@ -259,12 +283,21 @@ func findTarget(ctx context.Context, c *dbus.Conn, t Target) (dbus.ObjectPath, T
if e != nil {
return "", Target{}, e
}
- for _, ifs := range o {
+ for path, ifs := range o {
p := ifs["org.bluez.Device1"]
candidate, ok := advertised(p)
connected, _ := p["Connected"].Value().(bool)
if ok && connected && strings.EqualFold(candidate.UUID, t.UUID) && candidate.Adapter == t.Adapter {
- return "", Target{}, errors.New("Sesame is already connected through this adapter")
+ if candidate.Model != t.Model {
+ return "", Target{}, errors.New("Sesame model changed; scan again")
+ }
+ // Live preparations/clients never redial their own sessions. Reaching
+ // this point means this dialer has no token/cipher for the existing
+ // BlueZ link; reconnect just this device to obtain a fresh initial.
+ log.Printf("Sesame %s at %s on %s: recovering an existing BLE link before session setup", t.UUID, candidate.Address, t.Adapter)
+ if e := disconnectDevice(ctx, c, path); e != nil {
+ return "", Target{}, &setupRetryError{e}
+ }
}
}
adapter := c.Object("org.bluez", adapterPath)
@@ -329,7 +362,12 @@ func findTarget(ctx context.Context, c *dbus.Conn, t Target) (dbus.ObjectPath, T
}
connected, _ := o[path]["org.bluez.Device1"]["Connected"].Value().(bool)
if connected {
- return "", Target{}, errors.New("Sesame is already connected through this adapter")
+ // A connection may complete while we scan (for example a
+ // late completion of an earlier canceled Connect call).
+ if e := disconnectDevice(ctx, c, path); e != nil {
+ return "", Target{}, &setupRetryError{e}
+ }
+ return "", Target{}, &setupRetryError{errors.New("recovered a BLE link established during discovery; rescan for a fresh session")}
}
return path, found, nil
}
diff --git a/drivers/sesame/bluez_test.go b/drivers/sesame/bluez_test.go
index 7dd6015..f8abc69 100644
--- a/drivers/sesame/bluez_test.go
+++ b/drivers/sesame/bluez_test.go
@@ -17,13 +17,16 @@ import (
// A private bus exercises the actual transport, without a controller or BlueZ.
type testBlueZ struct {
- mu sync.Mutex
- conn *dbus.Conn
- objects objects
- calls []string
- attempts int
- resolutionDelay time.Duration
- readyAt time.Time
+ mu sync.Mutex
+ conn *dbus.Conn
+ objects objects
+ calls []string
+ attempts int
+ resolutionDelay time.Duration
+ readyAt time.Time
+ disconnectDelay time.Duration
+ disconnectAt time.Time
+ connectDuringScan bool
}
const testDevice dbus.ObjectPath = "/org/bluez/hci0/dev_FF_FF_FF_FF_FF_FF"
@@ -34,6 +37,10 @@ const testNotify dbus.ObjectPath = testService + "/char0003"
func (b *testBlueZ) GetManagedObjects() (objects, *dbus.Error) {
b.mu.Lock()
defer b.mu.Unlock()
+ if !b.disconnectAt.IsZero() && !time.Now().Before(b.disconnectAt) {
+ b.objects[testDevice]["org.bluez.Device1"]["Connected"] = dbus.MakeVariant(false)
+ b.disconnectAt = time.Time{}
+ }
if !b.readyAt.IsZero() && !time.Now().Before(b.readyAt) {
b.objects[testDevice]["org.bluez.Device1"]["ServicesResolved"] = dbus.MakeVariant(true)
}
@@ -61,6 +68,10 @@ func (b *testBlueZ) SetDiscoveryFilter(filter map[string]dbus.Variant) *dbus.Err
func (b *testBlueZ) StartDiscovery() *dbus.Error {
b.mu.Lock()
b.calls = append(b.calls, "scan")
+ if b.connectDuringScan {
+ b.objects[testDevice]["org.bluez.Device1"]["Connected"] = dbus.MakeVariant(true)
+ b.connectDuringScan = false
+ }
b.mu.Unlock()
// Report unchanged data: a stale BlueZ entry must become eligible only here.
_ = b.conn.Emit(testDevice, "org.freedesktop.DBus.Properties.PropertiesChanged", "org.bluez.Device1", map[string]dbus.Variant{"RSSI": dbus.MakeVariant(int16(-45))}, []string{})
@@ -71,6 +82,9 @@ func (b *testBlueZ) Connect() *dbus.Error {
b.mu.Lock()
defer b.mu.Unlock()
b.calls = append(b.calls, "connect")
+ if connected, _ := b.objects[testDevice]["org.bluez.Device1"]["Connected"].Value().(bool); connected {
+ return dbus.NewError("org.bluez.Error.Failed", []interface{}{"Connect called before previous link disconnected"})
+ }
b.attempts++
if b.attempts == 1 {
return dbus.NewError("org.bluez.Error.Failed", []interface{}{"le-connection-abort-by-local"})
@@ -84,7 +98,11 @@ func (b *testBlueZ) Disconnect() *dbus.Error {
b.mu.Lock()
defer b.mu.Unlock()
b.calls = append(b.calls, "disconnect")
- b.objects[testDevice]["org.bluez.Device1"]["Connected"] = dbus.MakeVariant(false)
+ if connected, _ := b.objects[testDevice]["org.bluez.Device1"]["Connected"].Value().(bool); connected && b.disconnectDelay > 0 {
+ b.disconnectAt = time.Now().Add(b.disconnectDelay)
+ } else {
+ b.objects[testDevice]["org.bluez.Device1"]["Connected"] = dbus.MakeVariant(false)
+ }
return nil
}
func (b *testBlueZ) StartNotify() *dbus.Error {
@@ -111,10 +129,17 @@ func (b *testBlueZ) WriteValue(data []byte, options map[string]dbus.Variant) *db
return nil
}
func TestBlueZWorkflow(t *testing.T) {
- t.Run("immediate services", func(t *testing.T) { testBlueZWorkflow(t, 0) })
- t.Run("discovery exceeds old eight-second limit", func(t *testing.T) { testBlueZWorkflow(t, 8500*time.Millisecond) })
+ t.Run("immediate services", func(t *testing.T) { testBlueZWorkflow(t, 0, "") })
+ t.Run("discovery exceeds old eight-second limit", func(t *testing.T) { testBlueZWorkflow(t, 8500*time.Millisecond, "") })
+}
+func TestBlueZRecoversAlreadyConnectedLink(t *testing.T) {
+ testBlueZWorkflow(t, 0, "existing")
}
-func testBlueZWorkflow(t *testing.T, resolutionDelay time.Duration) {
+func TestBlueZRecoversConnectionCompletedDuringScan(t *testing.T) {
+ testBlueZWorkflow(t, 0, "during-scan")
+}
+func testBlueZWorkflow(t *testing.T, resolutionDelay time.Duration, mode string) {
+ alreadyConnected := mode == "existing"
daemon, e := exec.LookPath("dbus-daemon")
if e != nil {
t.Skip("dbus-daemon unavailable")
@@ -152,12 +177,16 @@ func testBlueZWorkflow(t *testing.T, resolutionDelay time.Duration) {
testDevice: {"org.bluez.Device1": {
"ManufacturerData": dbus.MakeVariant(map[uint16]dbus.Variant{0x055a: dbus.MakeVariant(advertisement)}),
"Address": dbus.MakeVariant("FF:FF:FF:FF:FF:FF"), "Adapter": dbus.MakeVariant(dbus.ObjectPath("/org/bluez/hci0")),
- "Connected": dbus.MakeVariant(false), "ServicesResolved": dbus.MakeVariant(false),
+ "Connected": dbus.MakeVariant(alreadyConnected), "ServicesResolved": dbus.MakeVariant(false),
}},
testService: {"org.bluez.GattService1": {"UUID": dbus.MakeVariant(ServiceUUID), "Device": dbus.MakeVariant(testDevice)}},
testWrite: {"org.bluez.GattCharacteristic1": {"UUID": dbus.MakeVariant(WriteUUID), "Service": dbus.MakeVariant(testService), "Flags": dbus.MakeVariant([]string{"write", "write-without-response"})}},
testNotify: {"org.bluez.GattCharacteristic1": {"UUID": dbus.MakeVariant(NotifyUUID), "Service": dbus.MakeVariant(testService)}},
}}
+ if mode != "" {
+ b.disconnectDelay = 150 * time.Millisecond
+ }
+ b.connectDuringScan = mode == "during-scan"
for path, iface := range map[dbus.ObjectPath]string{"/": "org.freedesktop.DBus.ObjectManager", "/org/bluez/hci0": "org.bluez.Adapter1", testDevice: "org.bluez.Device1", testWrite: "org.bluez.GattCharacteristic1", testNotify: "org.bluez.GattCharacteristic1"} {
if e := conn.Export(b, path, iface); e != nil {
t.Fatal(e)
@@ -189,6 +218,12 @@ func testBlueZWorkflow(t *testing.T, resolutionDelay time.Duration) {
b.mu.Lock()
defer b.mu.Unlock()
want := []string{"scan", "connect", "disconnect", "scan", "connect", "notify", "write", "stop-notify", "disconnect"}
+ if alreadyConnected {
+ want = append([]string{"disconnect"}, want...)
+ }
+ if mode == "during-scan" {
+ want = append([]string{"scan", "disconnect"}, want...)
+ }
if !reflect.DeepEqual(b.calls, want) {
t.Fatalf("calls %v; want %v", b.calls, want)
}
diff --git a/drivers/sesame/connections.go b/drivers/sesame/connections.go
index 1686818..5a3598e 100644
--- a/drivers/sesame/connections.go
+++ b/drivers/sesame/connections.go
@@ -82,7 +82,7 @@ func Disconnect(ctx context.Context, adapter, address string) error {
if !connected {
return nil
}
- return c.Object("org.bluez", path).CallWithContext(ctx, "org.bluez.Device1.Disconnect", 0).Err
+ return disconnectDevice(ctx, c, path)
}
return errors.New("Sesame device no longer found on this adapter; refresh connections")
}
diff --git a/ha-app/CHANGELOG.md b/ha-app/CHANGELOG.md
index febdd57..33ccff4 100644
--- a/ha-app/CHANGELOG.md
+++ b/ha-app/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.0.17
+
+- Fixed a bug that disconnect could report success after removing a failed preparation
+
## 0.0.16
- Follow official android app's workflow
diff --git a/ha-app/config.yaml b/ha-app/config.yaml
index 5238efa..84c784d 100644
--- a/ha-app/config.yaml
+++ b/ha-app/config.yaml
@@ -1,5 +1,5 @@
name: Home Key Go
-version: "0.0.16"
+version: "0.0.17"
slug: homekey_go
description: HomeKit virtual lock with Apple Home Key NFC authentication
url: https://sgit.k8s.astropenguin.net/penguin/homekey-go/
diff --git a/internal/app/physical.go b/internal/app/physical.go
index c9d08ce..af6d1ec 100644
--- a/internal/app/physical.go
+++ b/internal/app/physical.go
@@ -99,11 +99,7 @@ func physicalRoutes(mux *http.ServeMux, cfg *Settings) {
}
ctx, cancel := context.WithTimeout(r.Context(), 10*time.Second)
defer cancel()
- if cfg.Physical.DisconnectRegistration(r.PathValue("adapter"), r.PathValue("address")) {
- w.WriteHeader(http.StatusNoContent)
- return
- }
- if err := sesame.Disconnect(ctx, r.PathValue("adapter"), r.PathValue("address")); err != nil {
+ if err := disconnectSesame(ctx, cfg.Physical, r.PathValue("adapter"), r.PathValue("address"), sesame.Disconnect); err != nil {
http.Error(w, err.Error(), 409)
return
}
@@ -211,3 +207,10 @@ func physicalRoutes(mux *http.ServeMux, cfg *Settings) {
w.WriteHeader(http.StatusNoContent)
})
}
+
+// A failed preparation may never have acquired the BlueZ link. Retiring it
+// cannot replace releasing the actual device, even when its Close succeeds.
+func disconnectSesame(ctx context.Context, m *physical.Manager, adapter, address string, release func(context.Context, string, string) error) error {
+ m.DisconnectRegistration(adapter, address)
+ return release(ctx, adapter, address)
+}
diff --git a/internal/app/physical_bluez_test.go b/internal/app/physical_bluez_test.go
new file mode 100644
index 0000000..0462622
--- /dev/null
+++ b/internal/app/physical_bluez_test.go
@@ -0,0 +1,59 @@
+package app
+
+import (
+ "context"
+ "errors"
+ "testing"
+ "time"
+
+ "homekey.local/provisioner/drivers/sesame"
+ "homekey.local/provisioner/internal/physical"
+)
+
+// A failed preparation owns no Link. Retiring it must not short-circuit the
+// actual BlueZ disconnect, which was the reason repeated clicks were needed.
+func TestDisconnectFailedPreparationReleasesBlueZLink(t *testing.T) {
+ m, err := physical.Open(context.Background(), t.TempDir(), func(context.Context, sesame.Target) (sesame.Link, sesame.Target, error) {
+ return nil, sesame.Target{}, errors.New("Sesame is already connected through this adapter")
+ })
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer m.Close()
+ target := sesame.Target{UUID: "00112233-4455-6677-8899-aabbccddeeff", Model: "sesame_6_pro", Adapter: "hci0", Address: "AA:BB:CC:DD:EE:FF"}
+ m.Prepare([]sesame.Target{target})
+ deadline := time.Now().Add(time.Second)
+ for time.Now().Before(deadline) {
+ states := m.RegistrationConnections()
+ if len(states) == 1 && states[0].Phase == "failed" {
+ break
+ }
+ time.Sleep(time.Millisecond)
+ }
+ if states := m.RegistrationConnections(); len(states) != 1 || states[0].Phase != "failed" {
+ t.Fatal("preparation did not fail", states)
+ }
+ connected := true
+ calls := 0
+ release := func(ctx context.Context, adapter, address string) error {
+ if adapter != target.Adapter || address != target.Address {
+ t.Fatal("released wrong lock", adapter, address)
+ }
+ if len(m.RegistrationConnections()) != 0 {
+ t.Fatal("released before retiring preparation")
+ }
+ calls++
+ connected = false
+ return nil
+ }
+ if err = disconnectSesame(context.Background(), m, target.Adapter, target.Address, release); err != nil {
+ t.Fatal(err)
+ }
+ if connected || calls != 1 {
+ t.Fatal("failed preparation hid the live BlueZ link", connected, calls)
+ }
+ transportError := errors.New("disconnect failed")
+ if err = disconnectSesame(context.Background(), m, target.Adapter, target.Address, func(context.Context, string, string) error { return transportError }); !errors.Is(err, transportError) {
+ t.Fatal("hid transport disconnect failure", err)
+ }
+}