Chorus32-ESP32LapTimer icon indicating copy to clipboard operation
Chorus32-ESP32LapTimer copied to clipboard

For discussion: Extended commands

Open Smeat opened this issue 4 years ago • 5 comments

This adds a skeleton for sending extended commands over the normal connections.

This way we'll be able to completely control everything using the normal connections and will replace the json stuff from the webserver in the future and use websockets for the communication. This allows us to make the page much more responsive (no reloading required). The json way also consumes a large amount of dynamic memory, which should be avoided.

Still open for suggestions on how to improve this.

Test for compatability:

  • [X] Android app
  • [ ] ios app
  • [X] Racegod
  • [ ] Livetime

Smeat avatar Apr 03 '20 13:04 Smeat

I'm thinking it might be a good idea to be able to mark a client as supporting the extended format. Like say when a client connects it can send along a command to indicate that E is supported that way we can avoid sending a lot of excess data to clients that can't do anything with it.

I think this would only apply in a scenario where there are more than one clients connected though because i haven't seen any code that doles out data on a per client basis it goes to everyone all the time if more than one is connected. I could however be mistaken

swampmonster avatar Apr 04 '20 19:04 swampmonster

I added a short description on what these messages might do. I am using them in my fork, but just see them as a suggestion we might implement later.

Yeah at first only one client was supported on the protocol. When I added support for multiple clients I just made everything a broadcast. But as most messages are only sent upon request having a few extra messages for an unsupported client shouldn't be that bad.

Smeat avatar Apr 04 '20 19:04 Smeat

I added a short description on what these messages might do. I am using them in my fork, but just see them as a suggestion we might implement later.

Yeah at first only one client was supported on the protocol. When I added support for multiple clients I just made everything a broadcast. But as most messages are only sent upon request having a few extra messages for an unsupported client shouldn't be that bad.

You're right it's not that much extra data. I would like to not broadcast everything but that's for a different pull request :)

Edit: A concern might be what clients that don't support E does however. If they handle an unkown command gracefully

swampmonster avatar Apr 04 '20 19:04 swampmonster

If the implementation is correct, these messages should just be discarded. Not sure how you'd implement this wrong. This is also only a concern if you have >= 2 clients where one uses the extended commands. But this should definitely be tested before merging.

No problem when using the Android app.

Smeat avatar Apr 04 '20 19:04 Smeat

If the implementation is correct, these messages should just be discarded. Not sure how you'd implement this wrong. This is also only a concern if you have >= 2 clients where one uses the extended commands. But this should definitely be tested before merging.

No problem when using the Android app.

I’ll test it in the iOS app

swampmonster avatar Apr 05 '20 08:04 swampmonster