Repository: evcc-io/evcc
Pull request #19011 (2025-02-20 18:43:41)

Tibber Pulse: fix duplicate subscriptions
[According to graphql-client maintainers](https://github.com/hasura/go-graphql-client/issues/164#issuecomment-2672246361), resubscribing after client disconnect has become counterproductive after the recent changes.
Removing the re-subscribe should fix the duplicate subscriptions observed [here](https://github.com/evcc-io/evcc/issues/17925#issuecomment-2663935721).

<strike>@andig I used `go get github.com/hasura/go-graphql-client@5970b87` to update the client. Looking at the change set, I see there's now two entries for graphql in `go.sum`. Is this OK/correct, or should the previous entry be removed? If the latter, any reason why `go get` did not do so on its own?</strike>

Nevermind, broke porcelain - fixed it.

andig: Another great PR, thank you! Wondering: do we even need the ticker now? Is there anything stopping the client from running?

Patch:
diff --git a/core/loadpoint/api.go b/core/loadpoint/api.go
index 25a95bbb..83b6ef9d 100644
--- a/core/loadpoint/api.go
+++ b/core/loadpoint/api.go
@@ -116,7 +116,7 @@ type API interface {
 	// SocBasedPlanning determines if the planner is soc based
 	SocBasedPlanning() bool
 	// GetPlan creates a charging plan
-	GetPlan(targetTime time.Time, requiredDuration time.Duration) (api.Rates, error)
+	GetPlan(targetTime time.Time, requiredDuration time.Duration) api.Rates
 
 	// GetSocConfig returns the soc poll settings
 	GetSocConfig() SocConfig
diff --git a/core/loadpoint/mock.go b/core/loadpoint/mock.go
index 0f150589..620bdc20 100644
--- a/core/loadpoint/mock.go
+++ b/core/loadpoint/mock.go
@@ -420,12 +420,11 @@ func (mr *MockAPIMockRecorder) GetPhasesConfigured() *gomock.Call {
 }
 
 // GetPlan mocks base method.
-func (m *MockAPI) GetPlan(targetTime time.Time, requiredDuration time.Duration) (api.Rates, error) {
+func (m *MockAPI) GetPlan(targetTime time.Time, requiredDuration time.Duration) api.Rates {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "GetPlan", targetTime, requiredDuration)
 	ret0, _ := ret[0].(api.Rates)
-	ret1, _ := ret[1].(error)
-	return ret0, ret1
+	return ret0
 }
 
 // GetPlan indicates an expected call of GetPlan.
diff --git a/core/loadpoint_plan.go b/core/loadpoint_plan.go
index 60f6acf9..064990ae 100644
--- a/core/loadpoint_plan.go
+++ b/core/loadpoint_plan.go
@@ -79,9 +79,9 @@ func (lp *Loadpoint) GetPlanGoal() (float64, bool) {
 }
 
 // GetPlan creates a charging plan for given time and duration
-func (lp *Loadpoint) GetPlan(targetTime time.Time, requiredDuration time.Duration) (api.Rates, error) {
+func (lp *Loadpoint) GetPlan(targetTime time.Time, requiredDuration time.Duration) api.Rates {
 	if lp.planner == nil || targetTime.IsZero() {
-		return nil, nil
+		return nil
 	}
 
 	return lp.planner.Plan(requiredDuration, targetTime)
@@ -132,9 +132,8 @@ func (lp *Loadpoint) plannerActive() (active bool) {
 		return false
 	}
 
-	plan, err := lp.GetPlan(planTime, requiredDuration)
-	if err != nil {
-		lp.log.ERROR.Println("planner:", err)
+	plan := lp.GetPlan(planTime, requiredDuration)
+	if plan == nil {
 		return false
 	}
 
diff --git a/core/planner/planner.go b/core/planner/planner.go
index a9e3301f..cfa97605 100644
--- a/core/planner/planner.go
+++ b/core/planner/planner.go
@@ -136,9 +136,9 @@ func (t *Planner) continuousPlan(rates api.Rates, start, end time.Time) api.Rate
 	return res
 }
 
-func (t *Planner) Plan(requiredDuration time.Duration, targetTime time.Time) (api.Rates, error) {
+func (t *Planner) Plan(requiredDuration time.Duration, targetTime time.Time) api.Rates {
 	if t == nil || requiredDuration <= 0 {
-		return nil, nil
+		return nil
 	}
 
 	latestStart := targetTime.Add(-requiredDuration)
@@ -157,19 +157,19 @@ func (t *Planner) Plan(requiredDuration time.Duration, targetTime time.Time) (ap
 
 	// target charging without tariff or late start
 	if t.tariff == nil {
-		return simplePlan, nil
+		return simplePlan
 	}
 
 	rates, err := t.tariff.Rates()
 
 	// treat like normal target charging if we don't have rates
 	if len(rates) == 0 || err != nil {
-		return simplePlan, err
+		return simplePlan
 	}
 
 	// consume remaining time
 	if t.clock.Until(targetTime) <= requiredDuration {
-		return t.continuousPlan(rates, latestStart, targetTime), nil
+		return t.continuousPlan(rates, latestStart, targetTime)
 	}
 
 	// rates are by default sorted by date, oldest to newest
@@ -183,7 +183,7 @@ func (t *Planner) Plan(requiredDuration time.Duration, targetTime time.Time) (ap
 		// there is enough time for charging after end of current rates
 		durationAfterRates := targetTime.Sub(last)
 		if durationAfterRates >= requiredDuration {
-			return nil, nil
+			return nil
 		}
 
 		// need to use some of the available slots
@@ -199,5 +199,5 @@ func (t *Planner) Plan(requiredDuration time.Duration, targetTime time.Time) (ap
 	// sort plan by time
 	plan.Sort()
 
-	return plan, nil
+	return plan
 }
diff --git a/go.mod b/go.mod
index cb34bdab..c16d1106 100644
--- a/go.mod
+++ b/go.mod
@@ -48,7 +48,7 @@ require (
 	github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79
 	github.com/grid-x/modbus v0.0.0-20241004123532-f6c6fb5201b3
 	github.com/hashicorp/go-version v1.7.0
-	github.com/hasura/go-graphql-client v0.13.2-0.20250210080311-cf325bddb83b
+	github.com/hasura/go-graphql-client v0.13.2-0.20250219070609-5970b87363a3
 	github.com/influxdata/influxdb-client-go/v2 v2.14.0
 	github.com/insomniacslk/tapo v1.0.1
 	github.com/itchyny/gojq v0.12.17
diff --git a/go.sum b/go.sum
index 6ab363e1..72872cf8 100644
--- a/go.sum
+++ b/go.sum
@@ -329,8 +329,8 @@ github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO
 github.com/hashicorp/mdns v1.0.0/go.mod h1:tL+uN++7HEJ6SQLQ2/p+z2pH24WQKWjBPkE0mNTz8vQ=
 github.com/hashicorp/memberlist v0.1.3/go.mod h1:ajVTdAv/9Im8oMAAj5G31PhhMCZJV2pPBoIllUwCN7I=
 github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc=
-github.com/hasura/go-graphql-client v0.13.2-0.20250210080311-cf325bddb83b h1:TNn5zg7P6l9aCDjdlT+ryc35fmDbK888O7F4TTs/pFQ=
-github.com/hasura/go-graphql-client v0.13.2-0.20250210080311-cf325bddb83b/go.mod h1:k7FF7h53C+hSNFRG3++DdVZWIuHdCaTbI7siTJ//zGQ=
+github.com/hasura/go-graphql-client v0.13.2-0.20250219070609-5970b87363a3 h1:DsBHMwn8AqZRx/aChi3sSLriBKRnSlaWb28fY9QVzJ8=
+github.com/hasura/go-graphql-client v0.13.2-0.20250219070609-5970b87363a3/go.mod h1:k7FF7h53C+hSNFRG3++DdVZWIuHdCaTbI7siTJ//zGQ=
 github.com/hinshun/vt10x v0.0.0-20220119200601-820417d04eec h1:qv2VnGeEQHchGaZ/u7lxST/RaJw+cv273q79D81Xbog=
 github.com/hinshun/vt10x v0.0.0-20220119200601-820417d04eec/go.mod h1:Q48J4R4DvxnHolD5P8pOtXigYlRuPLGl6moFx3ulM68=
 github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
diff --git a/meter/tibber-pulse.go b/meter/tibber-pulse.go
index f395857a..9dedd9c4 100644
--- a/meter/tibber-pulse.go
+++ b/meter/tibber-pulse.go
@@ -20,8 +20,7 @@ func init() {
 }
 
 type Tibber struct {
-	data   *util.Monitor[tibber.LiveMeasurement]
-	homeID string
+	data *util.Monitor[tibber.LiveMeasurement]
 }
 
 func NewTibberFromConfig(ctx context.Context, other map[string]interface{}) (api.Meter, error) {
@@ -68,8 +67,7 @@ func NewTibberFromConfig(ctx context.Context, other map[string]interface{}) (api
 	}
 
 	t := &Tibber{
-		data:   util.NewMonitor[tibber.LiveMeasurement](cc.Timeout),
-		homeID: cc.HomeID,
+		data: util.NewMonitor[tibber.LiveMeasurement](cc.Timeout),
 	}
 
 	// subscription client
@@ -101,8 +99,18 @@ func NewTibberFromConfig(ctx context.Context, other map[string]interface{}) (api
 			return nil
 		})
 
-	if err := t.ensureSubscribed(client, cc.Timeout); err != nil {
-		return nil, err
+	done := make(chan error, 1)
+	go func(done chan error) {
+		done <- t.subscribe(client, cc.HomeID)
+	}(done)
+
+	select {
+	case err := <-done:
+		if err != nil {
+			return nil, err
+		}
+	case <-time.After(cc.Timeout):
+		return nil, api.ErrTimeout
 	}
 
 	go func() {
@@ -113,16 +121,11 @@ func NewTibberFromConfig(ctx context.Context, other map[string]interface{}) (api
 	}()
 
 	go func() {
+		// The pulse sometimes declines valid(!) subscription requests, and asks the client to disconnect.
+		// Therefore we need to restart the client when exiting gracefully upon server request
+		// https://github.com/evcc-io/evcc/issues/17925#issuecomment-2621458890
 		for tick := time.Tick(10 * time.Second); ; {
-			err := client.Run()
-			if err == nil {
-				// The pulse sometimes declines valid(!) subscription requests, and asks the client to disconnect.
-				// This invalidates the subscription, and therefore we resubscribe when exiting Run() gracefully
-				// upon server request.
-				// https://github.com/evcc-io/evcc/issues/17925#issuecomment-2621458890
-				err = t.ensureSubscribed(client, cc.Timeout)
-			}
-			if err != nil {
+			if err := client.Run(); err != nil {
 				log.ERROR.Println(err)
 			}
 
@@ -137,27 +140,13 @@ func NewTibberFromConfig(ctx context.Context, other map[string]interface{}) (api
 	return t, nil
 }
 
-func (t *Tibber) ensureSubscribed(client *graphql.SubscriptionClient, timeout time.Duration) error {
-	done := make(chan error, 1)
-	go func(done chan error) {
-		done <- t.subscribe(client)
-	}(done)
-
-	select {
-	case err := <-done:
-		return err
-	case <-time.After(timeout):
-		return api.ErrTimeout
-	}
-}
-
-func (t *Tibber) subscribe(client *graphql.SubscriptionClient) error {
+func (t *Tibber) subscribe(client *graphql.SubscriptionClient, homeID string) error {
 	var query struct {
 		tibber.LiveMeasurement `graphql:"liveMeasurement(homeId: $homeId)"`
 	}
 
 	_, err := client.Subscribe(&query, map[string]any{
-		"homeId": graphql.ID(t.homeID),
+		"homeId": graphql.ID(homeID),
 	}, func(data []byte, err error) error {
 		if err != nil {
 			return err
diff --git a/plugin/http.go b/plugin/http.go
index 917fa4b6..0a0b89fe 100644
--- a/plugin/http.go
+++ b/plugin/http.go
@@ -2,6 +2,7 @@ package plugin
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -60,6 +61,10 @@ func NewHTTPPluginFromConfig(ctx context.Context, other map[string]interface{})
 		return nil, err
 	}
 
+	if cc.URI == "" {
+		return nil, errors.New("missing uri")
+	}
+
 	log := contextLogger(ctx, util.NewLogger("http"))
 	p := NewHTTP(
 		log,
diff --git a/server/http_loadpoint_handler.go b/server/http_loadpoint_handler.go
index 63258187..a7def346 100644
--- a/server/http_loadpoint_handler.go
+++ b/server/http_loadpoint_handler.go
@@ -50,8 +50,8 @@ func planHandler(lp loadpoint.API) http.HandlerFunc {
 
 		goal, _ := lp.GetPlanGoal()
 		requiredDuration := lp.GetPlanRequiredDuration(goal, maxPower)
-		plan, err := lp.GetPlan(planTime, requiredDuration)
-		if err != nil {
+		plan := lp.GetPlan(planTime, requiredDuration)
+		if plan == nil {
 			w.WriteHeader(http.StatusBadRequest)
 			return
 		}
@@ -109,8 +109,8 @@ func staticPlanPreviewHandler(lp loadpoint.API) http.HandlerFunc {
 
 		maxPower := lp.EffectiveMaxPower()
 		requiredDuration := lp.GetPlanRequiredDuration(goal, maxPower)
-		plan, err := lp.GetPlan(planTime, requiredDuration)
-		if err != nil {
+		plan := lp.GetPlan(planTime, requiredDuration)
+		if plan == nil {
 			w.WriteHeader(http.StatusBadRequest)
 			return
 		}
@@ -162,8 +162,8 @@ func repeatingPlanPreviewHandler(lp loadpoint.API) http.HandlerFunc {
 
 		maxPower := lp.EffectiveMaxPower()
 		requiredDuration := lp.GetPlanRequiredDuration(soc, maxPower)
-		plan, err := lp.GetPlan(planTime, requiredDuration)
-		if err != nil {
+		plan := lp.GetPlan(planTime, requiredDuration)
+		if plan == nil {
 			w.WriteHeader(http.StatusBadRequest)
 			return
 		}


Test patch:
diff --git a/core/planner/planner_test.go b/core/planner/planner_test.go
index e767447b..e6897222 100644
--- a/core/planner/planner_test.go
+++ b/core/planner/planner_test.go
@@ -133,8 +133,29 @@ func TestNilTariff(t *testing.T) {
 		clock: clock,
 	}
 
-	plan, err := p.Plan(time.Hour, clock.Now().Add(30*time.Minute))
-	require.NoError(t, err)
+	plan := p.Plan(time.Hour, clock.Now().Add(30*time.Minute))
+	assert.Equal(t, api.Rates{
+		{
+			Start: clock.Now(),
+			End:   clock.Now().Add(60 * time.Minute),
+		},
+	}, plan, "expected simple plan")
+}
+
+func TestRatesError(t *testing.T) {
+	clock := clock.NewMock()
+	ctrl := gomock.NewController(t)
+
+	trf := api.NewMockTariff(ctrl)
+	trf.EXPECT().Rates().AnyTimes().Return(nil, api.ErrOutdated)
+
+	p := &Planner{
+		log:    util.NewLogger("foo"),
+		clock:  clock,
+		tariff: trf,
+	}
+
+	plan := p.Plan(time.Hour, clock.Now().Add(30*time.Minute))
 	assert.Equal(t, api.Rates{
 		{
 			Start: clock.Now(),
@@ -163,12 +184,10 @@ func TestFlatTariffTargetInThePast(t *testing.T) {
 		},
 	}
 
-	plan, err := p.Plan(time.Hour, clock.Now().Add(30*time.Minute))
-	require.NoError(t, err)
+	plan := p.Plan(time.Hour, clock.Now().Add(30*time.Minute))
 	assert.Equal(t, simplePlan, plan, "expected simple plan")
 
-	plan, err = p.Plan(time.Hour, clock.Now().Add(-30*time.Minute))
-	require.NoError(t, err)
+	plan = p.Plan(time.Hour, clock.Now().Add(-30*time.Minute))
 	assert.Equal(t, simplePlan, plan, "expected simple plan")
 }
 
@@ -189,14 +208,12 @@ func TestFlatTariffLongSlots(t *testing.T) {
 	// that slots are not longer than 1 hour and with that context this is not a problem
 
 	// expect 00:00-01:00 UTC
-	plan, err := p.Plan(time.Hour, clock.Now().Add(2*time.Hour))
-	require.NoError(t, err)
+	plan := p.Plan(time.Hour, clock.Now().Add(2*time.Hour))
 	assert.Equal(t, api.Rate{Start: clock.Now(), End: clock.Now().Add(time.Hour)}, SlotAt(clock.Now(), plan))
 	assert.Equal(t, api.Rate{}, SlotAt(clock.Now().Add(time.Hour), plan))
 
 	// expect 00:00-01:00 UTC
-	plan, err = p.Plan(time.Hour, clock.Now().Add(time.Hour))
-	require.NoError(t, err)
+	plan = p.Plan(time.Hour, clock.Now().Add(time.Hour))
 	assert.Equal(t, api.Rate{Start: clock.Now(), End: clock.Now().Add(time.Hour)}, SlotAt(clock.Now(), plan))
 }
 
@@ -213,12 +230,10 @@ func TestTargetAfterKnownPrices(t *testing.T) {
 		tariff: trf,
 	}
 
-	plan, err := p.Plan(40*time.Minute, clock.Now().Add(2*time.Hour)) // charge efficiency does not allow to test with 1h
-	require.NoError(t, err)
+	plan := p.Plan(40*time.Minute, clock.Now().Add(2*time.Hour)) // charge efficiency does not allow to test with 1h
 	assert.False(t, !SlotAt(clock.Now(), plan).IsZero(), "should not start if car can be charged completely after known prices ")
 
-	plan, err = p.Plan(2*time.Hour, clock.Now().Add(2*time.Hour))
-	require.NoError(t, err)
+	plan = p.Plan(2*time.Hour, clock.Now().Add(2*time.Hour))
 	assert.True(t, !SlotAt(clock.Now(), plan).IsZero(), "should start if car can not be charged completely after known prices ")
 }
 
@@ -242,12 +257,10 @@ func TestChargeAfterTargetTime(t *testing.T) {
 		},
 	}
 
-	plan, err := p.Plan(time.Hour, clock.Now())
-	require.NoError(t, err)
+	plan := p.Plan(time.Hour, clock.Now())
 	assert.Equal(t, simplePlan, plan, "expected simple plan")
 
-	plan, err = p.Plan(time.Hour, clock.Now().Add(-time.Hour))
-	require.NoError(t, err)
+	plan = p.Plan(time.Hour, clock.Now().Add(-time.Hour))
 	assert.Equal(t, simplePlan, plan, "expected simple plan")
 }
 
@@ -259,8 +272,7 @@ func TestContinuousPlanNoTariff(t *testing.T) {
 		clock: clock,
 	}
 
-	plan, err := p.Plan(time.Hour, clock.Now())
-	require.NoError(t, err)
+	plan := p.Plan(time.Hour, clock.Now())
 
 	// single-slot plan
 	assert.Len(t, plan, 1)
@@ -281,8 +293,7 @@ func TestContinuousPlan(t *testing.T) {
 		tariff: trf,
 	}
 
-	plan, err := p.Plan(150*time.Minute, clock.Now())
-	require.NoError(t, err)
+	plan := p.Plan(150*time.Minute, clock.Now())
 
 	// 3-slot plan
 	assert.Len(t, plan, 3)
@@ -301,8 +312,7 @@ func TestContinuousPlanOutsideRates(t *testing.T) {
 		tariff: trf,
 	}
 
-	plan, err := p.Plan(30*time.Minute, clock.Now())
-	require.NoError(t, err)
+	plan := p.Plan(30*time.Minute, clock.Now())
 
 	// 3-slot plan
 	assert.Len(t, plan, 1)
diff --git a/plugin/config_test.go b/plugin/config_test.go
index cef0ca16..dbbd686a 100644
--- a/plugin/config_test.go
+++ b/plugin/config_test.go
@@ -7,7 +7,35 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestConfig(t *testing.T) {
+func TestRequiredConfig(t *testing.T) {
+	var c Config
+
+	_, err := c.BoolGetter(context.TODO())
+	assert.Error(t, err)
+
+	_, err = c.IntSetter(context.TODO(), "foo")
+	assert.Error(t, err)
+
+	c = Config{
+		Source: "http",
+		Other:  map[string]any{"uri": "http://localhost"},
+	}
+
+	g, err := c.BoolGetter(context.TODO())
+	assert.NoError(t, err)
+	assert.NotNil(t, g)
+
+	s, err := c.IntSetter(context.TODO(), "foo")
+	assert.NoError(t, err)
+	assert.NotNil(t, s)
+
+	c = Config{Source: "foo"}
+
+	_, err = c.BoolGetter(context.TODO())
+	assert.Error(t, err)
+}
+
+func TestOptionalConfig(t *testing.T) {
 	var c *Config
 
 	g, err := c.BoolGetter(context.TODO())