Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ford: async soc refresh #1001

Merged
merged 9 commits into from
May 13, 2021
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
wip
  • Loading branch information
andig committed May 11, 2021
commit ab6ff3d513a4f33574b80639a5ef6ee0fffa267d
32 changes: 16 additions & 16 deletions internal/vehicle/ford.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ type Ford struct {
user, password, vin string
tokenSource oauth2.TokenSource
statusG func() (interface{}, error)
statusRefreshId string
statusRefreshStart time.Time
refreshId string
refreshTime time.Time
}

func init() {
Expand Down Expand Up @@ -76,7 +76,7 @@ func NewFordFromConfig(other map[string]interface{}) (api.Vehicle, error) {
}

v.statusG = provider.NewCached(func() (interface{}, error) {
return v.vehicleStatus()
return v.status()
}, cc.Cache).InterfaceGetter()

if err == nil && cc.VIN == "" {
Expand Down Expand Up @@ -198,13 +198,13 @@ func (v *Ford) vehicles() ([]string, error) {
return vehicles, err
}

// vehicleStatus performs a /status request to the Ford API and triggers a refresh if
// status performs a /status request to the Ford API and triggers a refresh if
// the received status is too old
func (v *Ford) vehicleStatus() (res fordVehicleStatus, err error) {
func (v *Ford) status() (res fordVehicleStatus, err error) {
// follow up requested refresh
if v.statusRefreshId != "" {
if res, err = v.refreshResult(); errors.Is(err, api.ErrMustRetry) && time.Since(v.statusRefreshStart) > fordRefreshTimeout {
v.statusRefreshId = ""
if v.refreshId != "" {
if res, err = v.refreshResult(); errors.Is(err, api.ErrMustRetry) && time.Since(v.refreshTime) > fordRefreshTimeout {
v.refreshId = ""
err = api.ErrTimeout
}

Expand All @@ -225,7 +225,7 @@ func (v *Ford) vehicleStatus() (res fordVehicleStatus, err error) {
if elapsed := time.Since(lastUpdate); err == nil && elapsed > fordOutdatedAfter {
v.log.DEBUG.Printf("vehicle status is outdated (age %v > %v), requesting refresh", elapsed, fordOutdatedAfter)

if err = v.requestRefresh(); err == nil {
if err = v.refreshRequest(); err == nil {
err = api.ErrMustRetry
}
}
Expand All @@ -236,7 +236,7 @@ func (v *Ford) vehicleStatus() (res fordVehicleStatus, err error) {

// refreshResult triggers an update if not already in progress, otherwise gets result
func (v *Ford) refreshResult() (res fordVehicleStatus, err error) {
uri := fmt.Sprintf("%s/api/vehicles/v3/%s/statusrefresh/%s", fordAPI, v.vin, v.statusRefreshId)
uri := fmt.Sprintf("%s/api/vehicles/v3/%s/statusrefresh/%s", fordAPI, v.vin, v.refreshId)

var req *http.Request
if req, err = v.request(http.MethodGet, uri); err == nil {
Expand All @@ -247,7 +247,7 @@ func (v *Ford) refreshResult() (res fordVehicleStatus, err error) {
// waiting for vehicle and the request needs to be repeated
if err == nil {
Copy link
Contributor

@MrJayC MrJayC May 11, 2021

Choose a reason for hiding this comment

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

v.refreshId muss zurückgesetzt werden wenn err != nil

Copy link
Member Author

Choose a reason for hiding this comment

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

Das ist ne gute Frage. Könnte bei der nächsten Iteration ja klappen (Netzwerkproblem)? Aktuell würde das weiter passieren bis nachgelagert ErrTimeout kommt. Was meinst Du?

Copy link
Contributor

Choose a reason for hiding this comment

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

Guter Punkt, dann muss aber die Bedingung für den Timeout geändert werden (Zeile 206).
if res, err = v.refreshResult(); err != nil && time.Since(v.refreshTime) > fordRefreshTimeout {

Copy link
Member Author

Choose a reason for hiding this comment

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

willst Du's Dir nochmal greifen?

Copy link
Contributor

Choose a reason for hiding this comment

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

ich kann in dein Repo nicht pushen - für die eine Zeile rentiert sich nicht einen neuen Pull-Request zu öffnen (oder es gibt einen Trick, den ich nicht kenne)

Copy link
Member Author

@andig andig May 13, 2021

Choose a reason for hiding this comment

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

Eigentlich einfach

	if err != nil && time.Since(v.refreshTime) > fordRefreshTimeout {
		v.refreshId = ""
		if errors.Is(err, api.ErrMustRetry) {
			err = api.ErrTimeout
		}
	}

oder?

Copy link
Member Author

Choose a reason for hiding this comment

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

auch nciht, aber jetzt hab ich das problem verstanden, mom

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, neuer Versuch- denke es ist einfacher alle Logik an einem Platz zu haben. Jetzt gibts keine Kopfschmerzen mehr ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

So ist es perfekt 👌
Ich würde sagen „das ist es“ (auf den PR bezogen). Ich zieh die letzte Version auf jeden Fall nochmal auf mein Produktivsystem zum Test, erwarte aber keine Fehler.

Copy link
Member Author

Choose a reason for hiding this comment

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

Top, dann rein damit!

if res.Status == 200 {
v.statusRefreshId = ""
v.refreshId = ""
} else {
err = api.ErrMustRetry
}
Expand All @@ -256,9 +256,9 @@ func (v *Ford) refreshResult() (res fordVehicleStatus, err error) {
return res, err
}

// requestRefresh requests Ford API to poll vehicle for updated data
// returns commandId to track the request and get the data after server received update from vehicle
func (v *Ford) requestRefresh() error {
// refreshRequest requests Ford API to poll vehicle for updated data
// commandId tracks the request to check for update
func (v *Ford) refreshRequest() error {
var resp struct {
CommandId string
}
Expand All @@ -270,8 +270,8 @@ func (v *Ford) requestRefresh() error {
}

if err == nil {
v.statusRefreshId = resp.CommandId
v.statusRefreshStart = time.Now()
v.refreshId = resp.CommandId
v.refreshTime = time.Now()

if resp.CommandId == "" {
err = errors.New("refresh failed")
Expand Down