Repository: evcc-io/evcc
Pull request #14396 (2024-06-15 18:30:10)

Ensure fatal errors are json-serializable
Fix https://github.com/evcc-io/evcc/issues/14374


Patch:
diff --git a/cmd/error.go b/cmd/error.go
index d2e17574..7744080c 100644
--- a/cmd/error.go
+++ b/cmd/error.go
@@ -28,6 +28,27 @@ const (
 	ClassSponsorship
 )
 
+// FatalError is an error that can be marshaled
+type FatalError struct {
+	err error
+}
+
+func (e *FatalError) Error() string {
+	return e.err.Error()
+}
+
+func (e FatalError) MarshalJSON() ([]byte, error) {
+	if je, ok := e.err.(json.Marshaler); ok {
+		return je.MarshalJSON()
+	}
+
+	return json.Marshal(struct {
+		Error string `json:"error"`
+	}{
+		Error: e.err.Error(),
+	})
+}
+
 // DeviceError indicates the specific device that failed
 type DeviceError struct {
 	Name string
diff --git a/cmd/helper.go b/cmd/helper.go
index a42a46c3..183f0051 100644
--- a/cmd/helper.go
+++ b/cmd/helper.go
@@ -75,23 +75,25 @@ func shutdownDoneC() <-chan struct{} {
 	return doneC
 }
 
-func wrapError(err error) error {
-	if err != nil {
-		var opErr *net.OpError
-		var pathErr *os.PathError
+func wrapFatalError(err error) error {
+	if err == nil {
+		return nil
+	}
+
+	var opErr *net.OpError
+	var pathErr *os.PathError
 
-		switch {
-		case errors.As(err, &opErr):
-			if opErr.Op == "listen" && strings.Contains(opErr.Error(), "address already in use") {
-				err = fmt.Errorf("could not open port- check that evcc is not already running (%w)", err)
-			}
+	switch {
+	case errors.As(err, &opErr):
+		if opErr.Op == "listen" && strings.Contains(opErr.Error(), "address already in use") {
+			err = fmt.Errorf("could not open port- check that evcc is not already running (%w)", err)
+		}
 
-		case errors.As(err, &pathErr):
-			if pathErr.Op == "remove" && strings.Contains(pathErr.Error(), "operation not permitted") {
-				err = fmt.Errorf("could not remove file- check that evcc is not already running (%w)", err)
-			}
+	case errors.As(err, &pathErr):
+		if pathErr.Op == "remove" && strings.Contains(pathErr.Error(), "operation not permitted") {
+			err = fmt.Errorf("could not remove file- check that evcc is not already running (%w)", err)
 		}
 	}
 
-	return err
+	return &FatalError{err}
 }
diff --git a/cmd/root.go b/cmd/root.go
index a7c17e83..ecdd9aa1 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -291,7 +291,7 @@ func runRoot(cmd *cobra.Command, args []string) {
 
 	if err != nil {
 		// improve error message
-		err = wrapError(err)
+		err = wrapFatalError(err)
 		valueChan <- util.Param{Key: keys.Fatal, Val: err}
 
 		// TODO stop reboot loop if user updates config (or show countdown in UI)
@@ -307,5 +307,5 @@ func runRoot(cmd *cobra.Command, args []string) {
 	// uds health check listener
 	go server.HealthListener(site)
 
-	log.FATAL.Println(wrapError(httpd.ListenAndServe()))
+	log.FATAL.Println(wrapFatalError(httpd.ListenAndServe()))
 }
diff --git a/core/loadpoint.go b/core/loadpoint.go
index 802cf70d..af6555bf 100644
--- a/core/loadpoint.go
+++ b/core/loadpoint.go
@@ -1149,9 +1149,6 @@ func (lp *Loadpoint) pvScalePhases(sitePower, minCurrent, maxCurrent float64) in
 		lp.publishTimer(phaseTimer, lp.Enable.Delay, phaseScale3p)
 
 		if elapsed := lp.clock.Since(lp.phaseTimer); elapsed >= lp.Enable.Delay {
-			if err := lp.scalePhases(3); err != nil {
-				lp.log.ERROR.Println(err)
-			}
 			if err := lp.scalePhases(3); err != nil {
 				lp.log.ERROR.Println(err)
 			}
diff --git a/core/site.go b/core/site.go
index f382a186..569f4988 100644
--- a/core/site.go
+++ b/core/site.go
@@ -97,7 +97,7 @@ type Site struct {
 	pvPower      float64         // PV power
 	batteryPower float64         // Battery charge power
 	batterySoc   float64         // Battery soc
-	batteryMode  api.BatteryMode // Battery mode
+	batteryMode  api.BatteryMode // Battery mode (runtime only, not persisted)
 
 	publishCache map[string]any // store last published values to avoid unnecessary republishing
 }
diff --git a/core/site_api.go b/core/site_api.go
index 757b7017..62ae175a 100644
--- a/core/site_api.go
+++ b/core/site_api.go
@@ -327,14 +327,14 @@ func (site *Site) GetTariff(tariff string) api.Tariff {
 	}
 }
 
-// GetBatteryControl returns the battery control mode
+// GetBatteryDischargeControl returns the battery control mode (no discharge only)
 func (site *Site) GetBatteryDischargeControl() bool {
 	site.RLock()
 	defer site.RUnlock()
 	return site.batteryDischargeControl
 }
 
-// SetBatteryControl sets the battery control mode
+// SetBatteryDischargeControl sets the battery control mode (no discharge only)
 func (site *Site) SetBatteryDischargeControl(val bool) error {
 	site.log.DEBUG.Println("set battery discharge control:", val)
 
diff --git a/core/site_battery.go b/core/site_battery.go
index f78225a2..186275d7 100644
--- a/core/site_battery.go
+++ b/core/site_battery.go
@@ -19,6 +19,12 @@ func (site *Site) GetBatteryMode() api.BatteryMode {
 	return site.batteryMode
 }
 
+// setBatteryMode sets the battery mode
+func (site *Site) setBatteryMode(batMode api.BatteryMode) {
+	site.batteryMode = batMode
+	site.publish(keys.BatteryMode, batMode)
+}
+
 // SetBatteryMode sets the battery mode
 func (site *Site) SetBatteryMode(batMode api.BatteryMode) {
 	site.Lock()
@@ -27,11 +33,12 @@ func (site *Site) SetBatteryMode(batMode api.BatteryMode) {
 	site.log.DEBUG.Println("set battery mode:", batMode)
 
 	if site.batteryMode != batMode {
-		site.batteryMode = batMode
-		site.publish(keys.BatteryMode, batMode)
+		site.setBatteryMode(batMode)
 	}
 }
 
+// applyBatteryMode applies the mode to each battery and updates
+// internal state if successful (requires lock)
 func (site *Site) applyBatteryMode(mode api.BatteryMode) error {
 	// update batteries
 	for _, meter := range site.batteryMeters {
@@ -43,7 +50,7 @@ func (site *Site) applyBatteryMode(mode api.BatteryMode) error {
 	}
 
 	// update state and publish
-	site.SetBatteryMode(mode)
+	site.setBatteryMode(mode)
 
 	return nil
 }
@@ -89,8 +96,10 @@ func (site *Site) updateBatteryMode() {
 	}
 
 	if batMode := site.GetBatteryMode(); mode != batMode {
+		site.Lock()
 		if err := site.applyBatteryMode(mode); err != nil {
 			site.log.ERROR.Println("battery mode:", err)
 		}
+		site.Unlock()
 	}
 }
diff --git a/provider/http.go b/provider/http.go
index 67730251..6ee6f0ee 100644
--- a/provider/http.go
+++ b/provider/http.go
@@ -7,14 +7,12 @@ import (
 	"net/http"
 	"strconv"
 	"strings"
-	"text/template"
 	"time"
 
 	"github.com/evcc-io/evcc/provider/pipeline"
 	"github.com/evcc-io/evcc/util"
 	"github.com/evcc-io/evcc/util/request"
 	"github.com/evcc-io/evcc/util/transport"
-	"github.com/go-sprout/sprout"
 	"github.com/gregjones/httpcache"
 	"github.com/jpfielding/go-http-digest/pkg/digest"
 )
@@ -95,14 +93,9 @@ func NewHTTPProviderFromConfig(other map[string]interface{}) (Provider, error) {
 
 // NewHTTP create HTTP provider
 func NewHTTP(log *util.Logger, method, uri string, insecure bool, scale float64, cache time.Duration) *HTTP {
-	url := util.DefaultScheme(uri, "http")
-	if strings.HasPrefix(url, "http") && !strings.HasPrefix(uri, "http") {
-		log.WARN.Printf("missing scheme for %s, assuming http", uri)
-	}
-
 	p := &HTTP{
 		Helper: request.NewHelper(log),
-		url:    url,
+		url:    uri,
 		method: method,
 		scale:  scale,
 		cache:  cache,
@@ -166,18 +159,10 @@ func (p *HTTP) request(url string, body string) ([]byte, error) {
 			b = strings.NewReader(body)
 		}
 
-		tmpl, err := template.New("url").Funcs(sprout.TxtFuncMap()).Parse(url)
-		if err != nil {
-			return nil, err
-		}
-
-		builder := new(strings.Builder)
-		if err := tmpl.Execute(builder, nil); err != nil {
-			return nil, err
-		}
+		url := util.DefaultScheme(url, "http")
 
 		// empty method becomes GET
-		req, err := request.New(p.method, builder.String(), b, p.headers)
+		req, err := request.New(p.method, url, b, p.headers)
 		if err != nil {
 			return []byte{}, err
 		}
@@ -194,7 +179,12 @@ var _ StringProvider = (*HTTP)(nil)
 // StringGetter sends string request
 func (p *HTTP) StringGetter() (func() (string, error), error) {
 	return func() (string, error) {
-		b, err := p.request(p.url, p.body)
+		url, err := setFormattedValue(p.url, "", "")
+		if err != nil {
+			return "", err
+		}
+
+		b, err := p.request(url, p.body)
 
 		if err == nil && p.pipeline != nil {
 			b, err = p.pipeline.Process(b)
diff --git a/tariff/pun.go b/tariff/pun.go
index 4f1538b8..1a2b3d0d 100644
--- a/tariff/pun.go
+++ b/tariff/pun.go
@@ -21,10 +21,8 @@ import (
 
 type Pun struct {
 	*embed
-	log     *util.Logger
-	charges float32
-	tax     float32
-	data    *util.Monitor[api.Rates]
+	log  *util.Logger
+	data *util.Monitor[api.Rates]
 }
 
 type NewDataSet struct {
@@ -51,23 +49,16 @@ func init() {
 }
 
 func NewPunFromConfig(other map[string]interface{}) (api.Tariff, error) {
-	cc := struct {
-		Charges float32
-		Tax     float32
-	}{
-		Charges: 0.0,
-		Tax:     0.0,
-	}
+	var cc embed
 
 	if err := util.DecodeOther(other, &cc); err != nil {
 		return nil, err
 	}
 
 	t := &Pun{
-		log:     util.NewLogger("pun"),
-		charges: cc.Charges,
-		tax:     cc.Tax,
-		data:    util.NewMonitor[api.Rates](2 * time.Hour),
+		log:   util.NewLogger("pun"),
+		embed: &cc,
+		data:  util.NewMonitor[api.Rates](2 * time.Hour),
 	}
 
 	done := make(chan error)
@@ -217,7 +208,7 @@ func (t *Pun) getData(day time.Time) (api.Rates, error) {
 		ar := api.Rate{
 			Start: start,
 			End:   end,
-			Price: (price/1000.0 + float64(t.charges)) * (1 + float64(t.tax)),
+			Price: t.totalPrice(price / 1e3),
 		}
 		data = append(data, ar)
 	}


Test patch:
diff --git a/provider/http_test.go b/provider/http_test.go
index 593d2cbe..7316c6d7 100644
--- a/provider/http_test.go
+++ b/provider/http_test.go
@@ -3,13 +3,11 @@ package provider
 import (
 	"net/http"
 	"net/http/httptest"
-	"net/url"
 	"testing"
 
 	"github.com/evcc-io/evcc/util"
 	"github.com/samber/lo"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"
 )
 
 type httpHandler struct {
@@ -23,40 +21,54 @@ func (h *httpHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	_, _ = w.Write([]byte(h.val))
 }
 
-func TestHttpGet(t *testing.T) {
-	h := new(httpHandler)
-	srv := httptest.NewServer(h)
-	defer srv.Close()
+func TestHttp(t *testing.T) {
+	suite.Run(t, new(httpTestSuite))
+}
 
-	uri := srv.URL + "/foo/bar"
-	p := NewHTTP(util.NewLogger("foo"), http.MethodGet, uri, false, 1, 0)
+type httpTestSuite struct {
+	suite.Suite
+	h   *httpHandler
+	srv *httptest.Server
+}
+
+func (suite *httpTestSuite) SetupSuite() {
+	suite.h = new(httpHandler)
+	suite.srv = httptest.NewServer(suite.h)
+}
+
+func (suite *httpTestSuite) TearDown() {
+	suite.srv.Close()
+}
 
-	uriUrl, _ := url.Parse(uri)
+func (suite *httpTestSuite) TestGet() {
+	uri := suite.srv.URL + "/foo/bar{{\"/baz\"}}"
+	p := NewHTTP(util.NewLogger("foo"), http.MethodGet, uri, false, 1, 0)
 
 	g, err := p.StringGetter()
-	require.NoError(t, err)
+	suite.Require().NoError(err)
 
 	res, err := g()
-	require.NoError(t, err)
-	assert.Equal(t, uriUrl.Path, h.req.URL.Path)
-	assert.Equal(t, h.val, res)
+	suite.Require().NoError(err)
+	suite.Require().Equal("/foo/bar/baz", suite.h.req.URL.String())
+	suite.Require().Equal(suite.h.val, res)
 }
 
-func TestHttpSet(t *testing.T) {
-	h := new(httpHandler)
-	srv := httptest.NewServer(h)
-	defer srv.Close()
-
-	uri := srv.URL + "/foo/bar?baz={{.baz}}"
+func (suite *httpTestSuite) TestSetQuery() {
+	uri := suite.srv.URL + "/foo/bar?baz={{.baz}}"
 	p := NewHTTP(util.NewLogger("foo"), http.MethodGet, uri, false, 1, 0)
 
-	uriUrl, _ := url.Parse(uri)
-
 	s, err := p.StringSetter("baz")
-	require.NoError(t, err)
+	suite.Require().NoError(err)
+	suite.Require().NoError(s("4711"))
+	suite.Require().Equal("/foo/bar?baz=4711", suite.h.req.URL.String())
+}
 
-	err = s("4711")
-	require.NoError(t, err)
-	assert.Equal(t, uriUrl.Path, h.req.URL.Path)
-	assert.Equal(t, "baz=4711", h.req.URL.RawQuery)
+func (suite *httpTestSuite) TestSetPath() {
+	uri := suite.srv.URL + "/foo/bar/{{.baz}}"
+	p := NewHTTP(util.NewLogger("foo"), http.MethodGet, uri, false, 1, 0)
+
+	s, err := p.StringSetter("baz")
+	suite.Require().NoError(err)
+	suite.Require().NoError(s("4711"))
+	suite.Require().Equal("/foo/bar/4711", suite.h.req.URL.String())
 }