Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 64 additions & 63 deletions vehicle/saic/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"encoding/json"
"fmt"
"net/http"
"sync"
"sync/atomic"
"time"

"github.com/evcc-io/evcc/api"
Expand All @@ -14,21 +14,28 @@ import (
"github.com/evcc-io/evcc/vehicle/saic/requests"
)

const (
StatRunning = iota
StatValid
StatInvalid
)

const (
RegionEU = "https://gateway-mg-eu.soimt.com/api.app/v1/"
RegionAU = "https://gateway-mg-au.soimt.com/api.app/v1/"
)

type ConcurrentRequest struct {
Status atomic.Int32
Result requests.ChargeStatus
Comment on lines +28 to +30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Concurrent access to Result is racy despite using atomic for Status

Status is atomic, but Result is a plain field written in doRepeatedRequest/repeatRequest and read in Status from multiple goroutines. Since Status and Result are not updated/read as a single atomic operation, this introduces a data race and readers can observe inconsistent Result. Please either protect both fields with a mutex, store Result in an atomic.Value, or use an atomically swapped pointer to an immutable Result snapshot so readers see a race-free, consistent view.

}

// API is an api.Vehicle implementation for SAIC cars
type API struct {
*request.Helper
identity *Identity
request ConcurrentRequest
log *util.Logger

mu sync.Mutex
running bool // background refresh in progress
valid bool // result holds a valid response
result requests.ChargeStatus // last valid response
}

// NewAPI creates a new vehicle
Expand All @@ -43,65 +50,57 @@ func NewAPI(log *util.Logger, identity *Identity) *API {
Decorator: requests.Decorate,
Base: v.Client.Transport,
}
v.request.Status.Store(StatInvalid)

return v
}

// store saves a valid response and clears the running flag
func (v *API) store(res requests.ChargeStatus) {
v.mu.Lock()
defer v.mu.Unlock()
v.result, v.valid, v.running = res, true, false
}

// last returns the last valid response, or ErrMustRetry until one is available
func (v *API) last() (requests.ChargeStatus, error) {
v.mu.Lock()
defer v.mu.Unlock()
if v.valid {
return v.result, nil
}
return v.result, api.ErrMustRetry
}

func (v *API) doRepeatedRequest(path string, event_id string) error {
token, err := v.identity.Token()
if err != nil {
v.request.Status.Store(StatInvalid)
return err
}

req, _ := requests.CreateRequest(
req, err := requests.CreateRequest(
v.identity.baseUrl,
path,
http.MethodGet,
"",
request.JSONContent,
token.AccessToken,
event_id)
if err != nil {
v.request.Status.Store(StatInvalid)
return err
}

var res requests.Answer[requests.ChargeStatus]
if _, err = doRequest(v, req, &res); err == nil {
v.store(res.Data)
_, err = doRequest(v, req, &res)
if err == nil {
v.request.Result = res.Data
v.request.Status.Store(StatValid)
} else if err != api.ErrMustRetry {
v.request.Status.Store(StatInvalid)
}
return err
}

// repeatRequest polls for the deferred answer in the background
// This is running concurrently
func (v *API) repeatRequest(path string, event_id string) {
// always clear running so a future query can start
defer func() {
v.mu.Lock()
v.running = false
v.mu.Unlock()
}()

for count := range 20 {
var count int

v.request.Status.Store(StatRunning)
for err := api.ErrMustRetry; err == api.ErrMustRetry && count < 20; {
time.Sleep(2 * time.Second)
v.log.TRACE.Printf("repeated query %d", count)
if err := v.doRepeatedRequest(path, event_id); err != api.ErrMustRetry {
return
}
err = v.doRepeatedRequest(path, event_id)
count++
}

// Make sure that we don't exit here with status running (probably after 20 tries).
// This would not allow us to ever do a query again.
v.request.Status.CompareAndSwap(StatRunning, StatInvalid)
}

func doRequest[T any](v *API, req *http.Request, result *requests.Answer[T]) (string, error) {
Expand Down Expand Up @@ -174,65 +173,67 @@ func (v *API) Wakeup(vin string) error {

// Status implements the /user/vehicles/<vin>/status api
func (v *API) Status(vin string) (requests.ChargeStatus, error) {
var zero requests.ChargeStatus

// refresh in progress, return last known value
v.mu.Lock()
running := v.running
v.mu.Unlock()
if running {
// Check if we are already running in the background
if v.request.Status.CompareAndSwap(StatValid, StatInvalid) {
v.log.TRACE.Printf("stored value found")
return v.request.Result, nil
}
if v.request.Status.Load() == StatRunning {
v.log.TRACE.Printf("query running")
return v.last()
return v.request.Result, api.ErrMustRetry
}
v.log.TRACE.Printf("StatInvalid. Starting query")

token, err := v.identity.Token()
if err != nil {
return zero, err
return v.request.Result, err
}

path := "vehicle/charging/mgmtData?vin=" + requests.Sha256(vin)
// get charging status of vehicle
req, _ := requests.CreateRequest(
req, err := requests.CreateRequest(
v.identity.baseUrl,
path,
http.MethodGet,
"",
request.JSONContent,
token.AccessToken,
"")
if err != nil {
return v.request.Result, err
}

var res requests.Answer[requests.ChargeStatus]
event_id, err := doRequest(v, req, &res)
if err != nil {
return zero, err
return v.request.Result, err
}

if event_id == "" {
v.log.TRACE.Printf("answer without event id")
return v.last()
v.log.TRACE.Printf("Answer without event ID")
return v.request.Result, api.ErrMustRetry
}

req, _ = requests.CreateRequest(
req, err = requests.CreateRequest(
v.identity.baseUrl,
path,
http.MethodGet,
"",
request.JSONContent,
token.AccessToken,
event_id)
if err != nil {
return v.request.Result, err
}

_, err = doRequest(v, req, &res)

// answer not yet available, keep polling in the background
if _, err = doRequest(v, req, &res); err == api.ErrMustRetry {
v.mu.Lock()
v.running = true
v.mu.Unlock()
v.log.TRACE.Printf("no answer yet, continuing in background")
// Continue checking....
if err == api.ErrMustRetry {
v.request.Status.Store(StatRunning)
v.log.TRACE.Printf("no answer yet, continuing status query in background")
go v.repeatRequest(path, event_id)
return v.last()
} else if err != nil {
return zero, err
}

v.store(res.Data)
return res.Data, nil
return res.Data, err
}
5 changes: 3 additions & 2 deletions vehicle/saic/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (v *Provider) Soc() (float64, error) {

val := res.ChrgMgmtData.BmsPackSOCDsp
if val > 1000 {
// v.status.Reset()
v.status.Reset()
return float64(val), fmt.Errorf("invalid raw soc value: %d: %w", val, api.ErrMustRetry)
}

Expand Down Expand Up @@ -104,8 +104,9 @@ func (v *Provider) Range() (int64, error) {
return 0, err
}
val := res.RvsChargeStatus.FuelRangeElec
if val < 10 {
if val < 10 || val > 10000 {
// Ok, 0 would be possible, but it's more likely that it's an invalid answer.
v.status.Reset()
return 0, api.ErrMustRetry
}
return val / 10, nil
Expand Down
Loading