Only parse messages when we're planning on emitting.

[jeremydk]

Fixes #19

Use the emit timer to gate whether or not we should do the work on received messages via the serial. Moves the emit rate to 250ms, which reduces perceived lag without flooding the BLE head units with messages.

Numbers won't match perfectly because all clients do some level of secondly averaging.

[ptx2]

Hey, I had a chance to try this on a real bike and can definitely feel the improvement.

I noticed that the timing of the serial messages from the bike is roughly like this:

elapsed time  message received

  0ms         cadence
100ms         power
200ms         something else - probably resistance?
300ms         cadence
400ms         power
500ms         something else - probably resistance?
600ms         cadence
700ms         power
800ms         something else - probably resistance?
900ms         cadence
1000ms        power
...

Do you see any downside to calling onStatsUpdate() immediately upon receiving cadence and also immediately upon receiving power?

It's more emits (2x) at an irregular rate (100ms and 200ms alternating) but it looked good here in a quick test with Zwift.

It seems that would give the minimal possible lag and another nice side effect would be that we could remove the Timer and not have to worry about dirty tracking?

[jeremydk]

You know, it’s probably safe.

When I ran without a rate limiter, I would occasionally see a desynchronization at the start of my ride that would correct over 30-45 minutes, but thinking through it, it was likely the flood of post-dated pedal events after sitting at 0 while connecting devices which we’ve now fixed.

Zwift does some amount of secondly smoothing anyway, so at some point we’re optimizing for the amount of work done on the eventloop.

On Sat, Oct 10, 2020 at 11:29 AM ptx2 notifications@github.com wrote:

Hey, I had a chance to try this on a real bike and can definitely feel the improvement.

I noticed that the timing of the serial messages from the bike is roughly like this:

elapsed time message received

0ms cadence 100ms power 200ms something else - probably resistance? 300ms cadence 400ms power 500ms something else - probably resistance? 600ms cadence 700ms power 800ms something else - probably resistance? 900ms cadence 1000ms power ...

Do you see any downside to calling onStatsUpdate() immediately upon receiving cadence and also immediately upon receiving power?

It's more emits (2x) at an irregular rate (100ms and 200ms alternating) but it looked good here in a quick test with Zwift.

It seems that would give the minimal possible lag and another nice side effect would be that we could remove the Timer and not have to worry about dirty tracking?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ptx2/gymnasticon/pull/20#issuecomment-706592016, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADAM6MYLPXWQXPKSBASXWRLSKCR2DANCNFSM4SK4EC4A .

[ptx2]

Ah, I'll bet that was it too. I just got done doing some more testing with sending immediately and it looks good. There's a slight but noticable increase in responsiveness on the power figure.

Did you want to update the PR? Or, if you're ok with me adding on to it, I can just push up what I have and then merge this?

[jeremydk]

Feel free to patch and merge. Really appreciate it.

On Sat, Oct 10, 2020 at 2:02 PM ptx2 notifications@github.com wrote:

Ah, I'll bet that was it too. I just got done doing some more testing with sending immediately and it looks good. There's a slight but noticable increase in responsiveness on the power figure.

Did you want to update the PR? Or, if you're ok with me adding on to it, I can just push up what I have and then merge this?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ptx2/gymnasticon/pull/20#issuecomment-706611008, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADAM6M5SVVZ2BBZ3M4D364DSKDDVTANCNFSM4SK4EC4A .