Vehicle (Nissan Ariya/Micra): use time at reception instead of backend time (#32456)
This commit is contained in:
parent
ac5d384e39
commit
555ec3df36
5 changed files with 196 additions and 13 deletions
|
|
@ -66,6 +66,18 @@ func (v *API) BatteryStatus(vin, version string) (StatusResponse, error) {
|
||||||
var res StatusResponse
|
var res StatusResponse
|
||||||
err := v.GetJSON(uri, &res)
|
err := v.GetJSON(uri, &res)
|
||||||
|
|
||||||
|
if err == nil {
|
||||||
|
res.Updated = time.Time{}
|
||||||
|
|
||||||
|
if version == "v1" && res.LastUpdateTime != nil {
|
||||||
|
res.Updated = res.LastUpdateTime.Time
|
||||||
|
}
|
||||||
|
|
||||||
|
if version == "v2" && res.Timestamp != nil {
|
||||||
|
res.Updated = time.Now()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return res, err
|
return res, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
96
vehicle/nissan/api_test.go
Normal file
96
vehicle/nissan/api_test.go
Normal file
|
|
@ -0,0 +1,96 @@
|
||||||
|
package nissan
|
||||||
|
|
||||||
|
import (
|
||||||
|
"io"
|
||||||
|
"net/http"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/evcc-io/evcc/util"
|
||||||
|
"github.com/evcc-io/evcc/util/request"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
type roundTripFunc func(*http.Request) (*http.Response, error)
|
||||||
|
|
||||||
|
func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }
|
||||||
|
|
||||||
|
func newTestAPI(body string) *API {
|
||||||
|
v := &API{Helper: request.NewHelper(util.NewLogger("test"))}
|
||||||
|
v.Client.Transport = roundTripFunc(func(*http.Request) (*http.Response, error) {
|
||||||
|
return &http.Response{
|
||||||
|
StatusCode: http.StatusOK,
|
||||||
|
Body: io.NopCloser(strings.NewReader(body)),
|
||||||
|
Header: http.Header{"Content-Type": {"application/json"}},
|
||||||
|
}, nil
|
||||||
|
})
|
||||||
|
return v
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestBatteryStatusV2IgnoresPastTimestamp verifies that BatteryStatus for v2
|
||||||
|
// ignores the raw timestamp value and sets Updated to roughly now, regardless
|
||||||
|
// of how old the reported timestamp is.
|
||||||
|
//
|
||||||
|
// Background: the Nissan Ariya (Kamereon v2) returns timestamps that are
|
||||||
|
// 30 min–7 h in the past. Using that raw value as Updated caused Provider.status
|
||||||
|
// to always consider the result stale (time.Since > 5 min expiry) and issue
|
||||||
|
// an endless stream of refresh requests. Setting Updated = time.Now() on a
|
||||||
|
// non-nil timestamp breaks that loop.
|
||||||
|
func TestBatteryStatusV2IgnoresPastTimestamp(t *testing.T) {
|
||||||
|
past30m := time.Now().Add(-30 * time.Minute)
|
||||||
|
past2h := time.Now().Add(-2 * time.Hour)
|
||||||
|
past7h := time.Now().Add(-7 * time.Hour)
|
||||||
|
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
timestamp time.Time
|
||||||
|
}{
|
||||||
|
{"30 minutes old", past30m},
|
||||||
|
{"2 hours old (mid-range)", past2h},
|
||||||
|
{"7 hours old (worst case)", past7h},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range cases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
body := `{"batteryLevel":80,"chargeStatus":1,"timestamp":"` + tc.timestamp.UTC().Format(time.RFC3339) + `"}`
|
||||||
|
res, err := newTestAPI(body).BatteryStatus("VIN", "v2")
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
// Updated must be close to now, not to the stale API timestamp.
|
||||||
|
assert.WithinDuration(t, time.Now(), res.Attributes.Updated, time.Minute,
|
||||||
|
"v2 Updated should be synthesized as now")
|
||||||
|
assert.False(t, res.Attributes.Updated.Equal(tc.timestamp),
|
||||||
|
"v2 must not use the raw past timestamp as Updated")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestBatteryStatusV2NoTimestampYieldsZero checks that a v2 response without
|
||||||
|
// a timestamp field leaves Updated as zero (no timestamp available at all).
|
||||||
|
func TestBatteryStatusV2NoTimestampYieldsZero(t *testing.T) {
|
||||||
|
body := `{"batteryLevel":80,"chargeStatus":1}`
|
||||||
|
res, err := newTestAPI(body).BatteryStatus("VIN", "v2")
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.True(t, res.Attributes.Updated.IsZero(), "v2 without timestamp should leave Updated zero")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestBatteryStatusV1HonorsPastTimestamp contrasts v2 behaviour: v1 uses
|
||||||
|
// lastUpdateTime directly, so a past timestamp is preserved in Updated.
|
||||||
|
func TestBatteryStatusV1HonorsPastTimestamp(t *testing.T) {
|
||||||
|
past2h := time.Now().Add(-2 * time.Hour).UTC().Truncate(time.Second)
|
||||||
|
body := `{"batteryLevel":80,"chargeStatus":1,"lastUpdateTime":"` + past2h.Format(timeFormat) + `"}`
|
||||||
|
res, err := newTestAPI(body).BatteryStatus("VIN", "v1")
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, past2h, res.Attributes.Updated, "v1 should use the reported lastUpdateTime verbatim")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestBatteryStatusV1NoTimestampYieldsZero checks that a v1 response without
|
||||||
|
// lastUpdateTime leaves Updated as zero.
|
||||||
|
func TestBatteryStatusV1NoTimestampYieldsZero(t *testing.T) {
|
||||||
|
body := `{"batteryLevel":80,"chargeStatus":1}`
|
||||||
|
res, err := newTestAPI(body).BatteryStatus("VIN", "v1")
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.True(t, res.Attributes.Updated.IsZero(), "v1 without lastUpdateTime should leave Updated zero")
|
||||||
|
}
|
||||||
|
|
@ -43,7 +43,7 @@ func (v *Provider) status(battery func() (StatusResponse, error), refresh func()
|
||||||
|
|
||||||
if err == nil {
|
if err == nil {
|
||||||
// result valid?
|
// result valid?
|
||||||
updated := res.Attributes.Updated()
|
updated := res.Attributes.Updated
|
||||||
if time.Since(updated) < v.expiry || updated.IsZero() {
|
if time.Since(updated) < v.expiry || updated.IsZero() {
|
||||||
v.refreshTime = time.Time{}
|
v.refreshTime = time.Time{}
|
||||||
return res, err
|
return res, err
|
||||||
|
|
@ -145,7 +145,7 @@ func (v *Provider) FinishTime() (time.Time, error) {
|
||||||
if res.Attributes.RemainingTime != nil {
|
if res.Attributes.RemainingTime != nil {
|
||||||
minutes := time.Duration(*res.Attributes.RemainingTime) * time.Minute
|
minutes := time.Duration(*res.Attributes.RemainingTime) * time.Minute
|
||||||
|
|
||||||
updated := res.Attributes.Updated()
|
updated := res.Attributes.Updated
|
||||||
if !updated.IsZero() {
|
if !updated.IsZero() {
|
||||||
return updated.Add(minutes), nil
|
return updated.Add(minutes), nil
|
||||||
}
|
}
|
||||||
|
|
|
||||||
84
vehicle/nissan/provider_test.go
Normal file
84
vehicle/nissan/provider_test.go
Normal file
|
|
@ -0,0 +1,84 @@
|
||||||
|
package nissan
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/evcc-io/evcc/api"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
// batteryFunc returns a StatusResponse with the given Updated time and
|
||||||
|
// records how many times it was called.
|
||||||
|
func batteryFunc(updated time.Time) (func() (StatusResponse, error), *int) {
|
||||||
|
calls := 0
|
||||||
|
return func() (StatusResponse, error) {
|
||||||
|
calls++
|
||||||
|
res := StatusResponse{}
|
||||||
|
res.Attributes.Updated = updated
|
||||||
|
return res, nil
|
||||||
|
}, &calls
|
||||||
|
}
|
||||||
|
|
||||||
|
// refreshFunc records calls and returns nil.
|
||||||
|
func refreshFunc() (func() (ActionResponse, error), *int) {
|
||||||
|
calls := 0
|
||||||
|
return func() (ActionResponse, error) {
|
||||||
|
calls++
|
||||||
|
return ActionResponse{}, nil
|
||||||
|
}, &calls
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestStatusV2SynthesizedNowSkipsRefresh is the core regression guard.
|
||||||
|
//
|
||||||
|
// When the Nissan Ariya (v2) API returns a timestamp 30 min–7 h in the past,
|
||||||
|
// BatteryStatus synthesizes Updated = time.Now(). Provider.status must treat
|
||||||
|
// that as fresh data (time.Since < 5 min expiry) and return the result
|
||||||
|
// immediately without calling the refresh endpoint.
|
||||||
|
func TestStatusV2SynthesizedNowSkipsRefresh(t *testing.T) {
|
||||||
|
p := &Provider{expiry: 5 * time.Minute}
|
||||||
|
|
||||||
|
battery, batteryCalls := batteryFunc(time.Now())
|
||||||
|
refresh, refreshCalls := refreshFunc()
|
||||||
|
|
||||||
|
res, err := p.status(battery, refresh)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, 1, *batteryCalls, "battery should be called once")
|
||||||
|
assert.Equal(t, 0, *refreshCalls, "refresh must not be called when data is fresh")
|
||||||
|
assert.Equal(t, 0, res.Attributes.BatteryLevel)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestStatusZeroTimestampSkipsRefresh verifies that a zero Updated (no
|
||||||
|
// timestamp in the API response at all) is treated as valid, matching the
|
||||||
|
// `updated.IsZero()` branch in Provider.status.
|
||||||
|
func TestStatusZeroTimestampSkipsRefresh(t *testing.T) {
|
||||||
|
p := &Provider{expiry: 5 * time.Minute}
|
||||||
|
|
||||||
|
battery, batteryCalls := batteryFunc(time.Time{})
|
||||||
|
refresh, refreshCalls := refreshFunc()
|
||||||
|
|
||||||
|
_, err := p.status(battery, refresh)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, 1, *batteryCalls)
|
||||||
|
assert.Equal(t, 0, *refreshCalls, "refresh must not be called when Updated is zero")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestStatusRawPastTimestampTriggersRefresh documents the bug that the
|
||||||
|
// timestamp fix resolved: if Updated held the raw Kamereon v2 timestamp
|
||||||
|
// (30 min–7 h old), time.Since(updated) > 5 min expiry, so every poll would
|
||||||
|
// trigger a refresh request.
|
||||||
|
func TestStatusRawPastTimestampTriggersRefresh(t *testing.T) {
|
||||||
|
p := &Provider{expiry: 5 * time.Minute}
|
||||||
|
|
||||||
|
// Simulate what the old Updated() method returned for v2: the raw API timestamp.
|
||||||
|
past2h := time.Now().Add(-2 * time.Hour)
|
||||||
|
battery, batteryCalls := batteryFunc(past2h)
|
||||||
|
refresh, refreshCalls := refreshFunc()
|
||||||
|
|
||||||
|
_, err := p.status(battery, refresh)
|
||||||
|
require.ErrorIs(t, err, api.ErrMustRetry,
|
||||||
|
"stale Updated (2 h old) should trigger a refresh and return ErrMustRetry")
|
||||||
|
assert.Equal(t, 1, *batteryCalls)
|
||||||
|
assert.Equal(t, 1, *refreshCalls, "refresh must be called when Updated is beyond expiry")
|
||||||
|
}
|
||||||
|
|
@ -92,17 +92,8 @@ type Attributes struct {
|
||||||
// v2
|
// v2
|
||||||
Timestamp *time.Time `json:"timestamp"`
|
Timestamp *time.Time `json:"timestamp"`
|
||||||
BatteryAutonomy *int `json:"batteryAutonomy"`
|
BatteryAutonomy *int `json:"batteryAutonomy"`
|
||||||
}
|
// synthesized fields
|
||||||
|
Updated time.Time
|
||||||
func (a *Attributes) Updated() time.Time {
|
|
||||||
if a.LastUpdateTime != nil {
|
|
||||||
// v1
|
|
||||||
return a.LastUpdateTime.Time
|
|
||||||
} else if a.Timestamp != nil {
|
|
||||||
// v2
|
|
||||||
return *a.Timestamp
|
|
||||||
}
|
|
||||||
return time.Time{}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
type ActionResponse struct {
|
type ActionResponse struct {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue