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

add "claims_version" event sent at each claims change. #474

Merged
merged 15 commits into from
Nov 17, 2022

Conversation

KipK
Copy link
Collaborator

@KipK KipK commented Nov 12, 2022

throw claims_version event at each claim change , incrementing from 0 to 254

@KipK
Copy link
Collaborator Author

KipK commented Nov 12, 2022

I don't remember where we've talked about this alrady. Also, I'm starting to need that for other cases in the UI.

Copy link
Collaborator

@jeremypoulter jeremypoulter left a comment

Choose a reason for hiding this comment

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

Looks good, will test

@KipK
Copy link
Collaborator Author

KipK commented Nov 13, 2022

I see a problem with current implementation.
As Shaper is using claims and modulate quiet often, this throws a lot of new claims_version, deserving the goal of limiting poll traffic.
there's no need of getting those shaper/divert claims updates

We need to find a better logic for the claims_version

@jeremypoulter
Copy link
Collaborator

I guess there are arguments for eventing only on additions/deletions of claims and skipping updates of existing claims. In all honesty do what best suits the v2 UI use case and we can fix any additional use cases if/when they come up. Really we need to update to a proper pub/sub protocol on the websocket I think.

@KipK
Copy link
Collaborator Author

KipK commented Nov 13, 2022

I guess there are arguments for eventing only on additions/deletions of claims and skipping updates of existing claims. In all honesty do what best suits the v2 UI use case and we can fix any additional use cases if/when they come up. Really we need to update to a proper pub/sub protocol on the websocket I think.

Extending websocket to a proper pub/sub protocol would simplify a lot the client logics.
This will need first to have a proper stored data implementation on ESP32 to handle event on data change only.

Btw, the Svelte "store" engine I use in UI 2 is already pub/sub based on client side.
All the data are coming in global stores and dispatched to subscribed components, a bit like Redux without the headaches

Keeping the same flux to the client/server implementation would cleanup a lot of code.

@KipK
Copy link
Collaborator Author

KipK commented Nov 14, 2022

So for now it updates the claims_version only for change coming from other clients than divert & shaper.

src/evse_man.cpp Outdated Show resolved Hide resolved
src/evse_man.cpp Outdated Show resolved Hide resolved
@KipK
Copy link
Collaborator Author

KipK commented Nov 14, 2022

I've changed to what you said, only increment when there's a state field in the key

@KipK
Copy link
Collaborator Author

KipK commented Nov 17, 2022

lucky you hadn't merged it yet, it introduced the bug I was talking about in the other PR.
It's fixed now

@jeremypoulter
Copy link
Collaborator

@KipK Please could you take a look at the conflict?

added ota event pushed on ota start / end ( throw "started", "completed", "failed" )
removed useless if condition
fix override topic not publishing props if there's no state prop
removed useless if condition
@KipK
Copy link
Collaborator Author

KipK commented Nov 17, 2022

mhh not sure of what happened here

@jeremypoulter jeremypoulter merged commit 0e6fc30 into OpenEVSE:master Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants