diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e2bc40361..862d26884f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ For details about compatibility between different releases, see the **Commitment ### Fixed +- Basic Station time synchronization no longer disables LNS-initiated time transfers when the gateway sends a `timesync` request. All Basic Station gateways send `timesync` regardless of PPS availability; disabling transfers prevented non-GPS gateways from receiving time correlation updates. +- LNS-initiated time transfers now include `xtime` and `gpstime` from recent uplinks, enabling direct concentrator-to-GPS time mapping per the Basic Station protocol specification. + ### Security ## [3.36.0] - unreleased diff --git a/pkg/gatewayserver/io/semtechws/lbslns/downstream_test.go b/pkg/gatewayserver/io/semtechws/lbslns/downstream_test.go index c460fc70d2..5ac6d8cda2 100644 --- a/pkg/gatewayserver/io/semtechws/lbslns/downstream_test.go +++ b/pkg/gatewayserver/io/semtechws/lbslns/downstream_test.go @@ -305,3 +305,112 @@ func TestTransferTime(t *testing.T) { a.So(res.MuxTime, should.Equal, semtechws.TimeToUnixSeconds(now)) } } + +func TestTimeSyncDoesNotDisableTransferTime(t *testing.T) { + a, ctx := test.New(t) + ctx = semtechws.NewContextWithSession(ctx, &semtechws.Session{}) + + f := (*lbsLNS)(nil) + now := time.Unix(500, 0) + + // Enable timesync for the session (as version.go does on connect). + semtechws.UpdateSessionTimeSync(ctx, true) + semtechws.UpdateSessionID(ctx, 0x42) + + // Process a timesync request (as upstream.go HandleUp does). + // Previously this called UpdateSessionTimeSync(ctx, false), which + // disabled LNS-initiated time transfers. After the fix, time sync + // should remain enabled. + req := TimeSyncRequest{TxTime: 123.456} + resp := req.Response(now) + a.So(resp.TxTime, should.Equal, 123.456) + + // Verify time sync is still enabled. + enabled, ok := semtechws.GetSessionTimeSync(ctx) + if !a.So(ok, should.BeTrue) { + t.FailNow() + } + a.So(enabled, should.BeTrue) + + // Verify TransferTime still produces output (not suppressed). + b, err := f.TransferTime(ctx, now, nil, nil) + if !a.So(err, should.BeNil) { + t.FailNow() + } + if !a.So(b, should.NotBeNil) { + t.Fatal("TransferTime returned nil after timesync request; time transfers were incorrectly disabled") + } + + var res TimeSyncResponse + if err := json.Unmarshal(b, &res); !a.So(err, should.BeNil) { + t.FailNow() + } + a.So(res.MuxTime, should.Equal, semtechws.TimeToUnixSeconds(now)) +} + +func TestTransferTimeWithUplinkXTime(t *testing.T) { + a, ctx := test.New(t) + ctx = semtechws.NewContextWithSession(ctx, &semtechws.Session{}) + + f := (*lbsLNS)(nil) + now := time.Unix(600, 0) + + semtechws.UpdateSessionTimeSync(ctx, true) + semtechws.UpdateSessionID(ctx, 0x42) + + // Store uplink timing from a GPS-equipped gateway. + xtime := int64(12666373963464220) + gpstime := int64(1232095200000000) + semtechws.UpdateLastUplink(ctx, xtime, gpstime, now) + + // Retrieve and use in time transfer (as ws.go ticker does). + gotXTime, gotGPSTime, rxAt, ok := semtechws.GetLastUplink(ctx) + if !a.So(ok, should.BeTrue) { + t.FailNow() + } + a.So(gotXTime, should.Equal, xtime) + a.So(gotGPSTime, should.Equal, gpstime) + a.So(rxAt, should.Equal, now) + + // Build TransferTime args from uplink data (simulating the ws.go ticker logic). + ct := semtechws.ConcentratorTimeFromXTime(gotXTime) + gt := semtechws.TimeFromGPSTime(gotGPSTime) + + b, err := f.TransferTime(ctx, now, >, &ct) + if !a.So(err, should.BeNil) { + t.FailNow() + } + if a.So(b, should.NotBeNil) { + var res TimeSyncResponse + if err := json.Unmarshal(b, &res); !a.So(err, should.BeNil) { + t.FailNow() + } + a.So(res.GPSTime, should.Equal, semtechws.TimeToGPSTime(gt)) + a.So(res.XTime, should.NotEqual, 0) + a.So(res.MuxTime, should.Equal, semtechws.TimeToUnixSeconds(now)) + } + + // Non-GPS gateway: gpstime=0, only MuxTime should be present. + // TransferTime requires both gpsTime and concentratorTime to produce + // xtime+gpstime; with nil gpsTime it falls back to MuxTime only. + semtechws.UpdateLastUplink(ctx, xtime, 0, now) + _, gotGPSTime, _, ok = semtechws.GetLastUplink(ctx) + if !a.So(ok, should.BeTrue) { + t.FailNow() + } + a.So(gotGPSTime, should.Equal, 0) + + b, err = f.TransferTime(ctx, now, nil, &ct) + if !a.So(err, should.BeNil) { + t.FailNow() + } + if a.So(b, should.NotBeNil) { + var res TimeSyncResponse + if err := json.Unmarshal(b, &res); !a.So(err, should.BeNil) { + t.FailNow() + } + a.So(res.GPSTime, should.Equal, 0) + a.So(res.XTime, should.Equal, 0) + a.So(res.MuxTime, should.Equal, semtechws.TimeToUnixSeconds(now)) + } +} diff --git a/pkg/gatewayserver/io/semtechws/lbslns/upstream.go b/pkg/gatewayserver/io/semtechws/lbslns/upstream.go index 9f19dc657f..7b5a731fea 100644 --- a/pkg/gatewayserver/io/semtechws/lbslns/upstream.go +++ b/pkg/gatewayserver/io/semtechws/lbslns/upstream.go @@ -601,6 +601,7 @@ func (f *lbsLNS) HandleUp( // nolint:gocyclo } semtechws.UpdateSessionID(ctx, semtechws.SessionIDFromXTime(jreq.UpInfo.XTime)) ct := recordTime(jreq.RefTime, jreq.UpInfo.XTime, jreq.UpInfo.GPSTime) + semtechws.UpdateLastUplink(ctx, jreq.UpInfo.XTime, jreq.UpInfo.GPSTime, receivedAt) if err := conn.HandleUp(up, ct); err != nil { logger.WithError(err).Warn("Failed to handle upstream message") } @@ -625,6 +626,7 @@ func (f *lbsLNS) HandleUp( // nolint:gocyclo } semtechws.UpdateSessionID(ctx, semtechws.SessionIDFromXTime(updf.UpInfo.XTime)) ct := recordTime(updf.RefTime, updf.UpInfo.XTime, updf.UpInfo.GPSTime) + semtechws.UpdateLastUplink(ctx, updf.UpInfo.XTime, updf.UpInfo.GPSTime, receivedAt) if err := conn.HandleUp(up, ct); err != nil { logger.WithError(err).Warn("Failed to handle upstream message") } @@ -650,10 +652,10 @@ func (f *lbsLNS) HandleUp( // nolint:gocyclo syncClock(txConf.XTime, txConf.GPSTime, true) case TypeUpstreamTimeSync: - // If the gateway sends a `timesync` request, it means that it has access to a PPS - // source. As such, there is no point in doing time transfers with this particular - // gateway. - semtechws.UpdateSessionTimeSync(ctx, false) + // The gateway sends a timesync request in order to correlate its concentrator + // counter with GPS time via round-trip calculation. All Basic Station gateways + // send this regardless of PPS availability. Continue sending LNS-initiated time + // transfers for direct xtime->gpstime mapping. var req TimeSyncRequest if err := json.Unmarshal(raw, &req); err != nil { return nil, err diff --git a/pkg/gatewayserver/io/semtechws/state.go b/pkg/gatewayserver/io/semtechws/state.go index ad17fbbc49..670b1e31a9 100644 --- a/pkg/gatewayserver/io/semtechws/state.go +++ b/pkg/gatewayserver/io/semtechws/state.go @@ -16,12 +16,19 @@ package semtechws import ( "context" + "time" ) // state represents the LBS session state. type state struct { ID *int32 TimeSync *bool + // Uplink timing for LNS-initiated time transfers. + // Updated on each jreq/updf so the periodic time transfer ticker + // can include a recent xtime+gpstime pair. + LastUplinkXTime int64 + LastUplinkGPSTime int64 + LastUplinkReceivedAt time.Time } // updateState updates the session state. @@ -84,3 +91,39 @@ func GetSessionTimeSync(ctx context.Context) (enabled bool, ok bool) { }).(bool) return d, ok } + +// UpdateLastUplink stores the timing info from the most recent uplink +// for use in LNS-initiated time transfers. +func UpdateLastUplink(ctx context.Context, xtime, gpstime int64, receivedAt time.Time) { + updateState(ctx, func(st *state) { + st.LastUplinkXTime = xtime + st.LastUplinkGPSTime = gpstime + st.LastUplinkReceivedAt = receivedAt + }) +} + +// GetLastUplink returns the most recent uplink timing info. +// Returns xtime, gpstime, receivedAt, and ok (true if data is available). +func GetLastUplink(ctx context.Context) (xtime int64, gpstime int64, receivedAt time.Time, ok bool) { + result := getState(ctx, func(st *state) any { + if st.LastUplinkXTime != 0 { + return &lastUplinkInfo{ + xtime: st.LastUplinkXTime, + gpstime: st.LastUplinkGPSTime, + receivedAt: st.LastUplinkReceivedAt, + } + } + return nil + }) + if result == nil { + return 0, 0, time.Time{}, false + } + info := result.(*lastUplinkInfo) + return info.xtime, info.gpstime, info.receivedAt, true +} + +type lastUplinkInfo struct { + xtime int64 + gpstime int64 + receivedAt time.Time +} diff --git a/pkg/gatewayserver/io/semtechws/state_test.go b/pkg/gatewayserver/io/semtechws/state_test.go new file mode 100644 index 0000000000..9c679d0401 --- /dev/null +++ b/pkg/gatewayserver/io/semtechws/state_test.go @@ -0,0 +1,67 @@ +// Copyright © 2024 The Things Network Foundation, The Things Industries B.V. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package semtechws + +import ( + "testing" + "time" + + "go.thethings.network/lorawan-stack/v3/pkg/util/test" + "go.thethings.network/lorawan-stack/v3/pkg/util/test/assertions/should" +) + +func TestLastUplink(t *testing.T) { + a, ctx := test.New(t) + ctx = NewContextWithSession(ctx, &Session{}) + + // No uplink data initially. + _, _, _, ok := GetLastUplink(ctx) + a.So(ok, should.BeFalse) + + // Store uplink timing info. + now := time.Now().UTC() + xtime := int64(12666373963464220) + gpstime := int64(1232095200000000) + UpdateLastUplink(ctx, xtime, gpstime, now) + + // Retrieve and verify. + gotXTime, gotGPSTime, gotReceivedAt, ok := GetLastUplink(ctx) + if !a.So(ok, should.BeTrue) { + t.FailNow() + } + a.So(gotXTime, should.Equal, xtime) + a.So(gotGPSTime, should.Equal, gpstime) + a.So(gotReceivedAt, should.Equal, now) + + // Update with new uplink. + later := now.Add(5 * time.Second) + xtime2 := int64(12666373963564220) + UpdateLastUplink(ctx, xtime2, gpstime, later) + + gotXTime, _, gotReceivedAt, ok = GetLastUplink(ctx) + if !a.So(ok, should.BeTrue) { + t.FailNow() + } + a.So(gotXTime, should.Equal, xtime2) + a.So(gotReceivedAt, should.Equal, later) + + // Non-GPS gateway: gpstime=0 is stored and retrieved. + UpdateLastUplink(ctx, xtime, 0, now) + _, gotGPSTime, _, ok = GetLastUplink(ctx) + if !a.So(ok, should.BeTrue) { + t.FailNow() + } + a.So(gotGPSTime, should.Equal, 0) +} diff --git a/pkg/gatewayserver/io/semtechws/ws.go b/pkg/gatewayserver/io/semtechws/ws.go index 655fdc6fa5..00caf1077e 100644 --- a/pkg/gatewayserver/io/semtechws/ws.go +++ b/pkg/gatewayserver/io/semtechws/ws.go @@ -337,9 +337,23 @@ func (s *srv) handleTraffic(w http.ResponseWriter, r *http.Request) (err error) } logger.Debug("Server pong sent") case <-timeSyncTickerC: - // TODO: Use GPS timestamp from a overlapping frames. - // https://github.com/TheThingsNetwork/lorawan-stack/issues/4852 - b, err := s.formatter.TransferTime(ctx, time.Now(), nil, nil) + // Send LNS-initiated time transfer using xtime+gpstime from recent uplinks. + // References https://github.com/TheThingsNetwork/lorawan-stack/issues/4852 + var gpsTime *time.Time + var concentratorTime *scheduling.ConcentratorTime + if xtime, gpstime, rxAt, ok := GetLastUplink(ctx); ok { + if time.Since(rxAt) < 10*time.Second { + elapsed := time.Since(rxAt) + extrapolatedXTime := xtime + int64(elapsed/time.Microsecond) + ct := ConcentratorTimeFromXTime(extrapolatedXTime) + concentratorTime = &ct + if gpstime != 0 { + gt := TimeFromGPSTime(gpstime).Add(elapsed) + gpsTime = > + } + } + } + b, err := s.formatter.TransferTime(ctx, time.Now(), gpsTime, concentratorTime) if err != nil { logger.WithError(err).Warn("Failed to generate time transfer") return err