Peloton Direct Polling support #43
Conversation
src/bikes/peloton.js
Outdated
| const PACKET_DELIMITER = Buffer.from('f6', 'hex'); | ||
| const POLL_RATE = 250; |
There was a problem hiding this comment.
Have you observed any issues with the poll rate being exceeded? Is there any benefit in making this value a configurable cli argument?
There was a problem hiding this comment.
It might eventually be configurable -- I've only tested for a few minutes at the moment to validate, but I know that there have been concerns around drift and lockup over long periods. Left this in as a way to help debug/investigate that behavior if we get a repro. I suspect the serialPort.drain() actually covers the underlying cause, but will need data to show that.
src/bikes/peloton.js
Outdated
| debuglog(`Measurement receive trigger: ${this.receiveTrigger}`); | ||
| if (this.receiveTrigger === RECEIVE_TRIGGER.POLL) { | ||
| if ((Object.entries(MEASUREMENTS_HEX_ENUM).length * SERIAL_WRITE_TIMEOUT) > POLL_RATE) { | ||
| throw new Error("Max Serial Write Timeout is longer than Poll Rate.") |
There was a problem hiding this comment.
Does this abort the gymnasticon process and one would need to restart it? Wondering if this should be just logged as a warning instead unless a one off timeout materially impacts the zwift experience?
There was a problem hiding this comment.
Yes, my expectation is that this is fatal. It's a subtle misconfiguration, but this keeps the peloton emitting on schedule. Without this check, assuming we were in a case where our serial writes were slow (near our timeout values), we could theoretically still be executing when we queue the next run of the poller. While node handles this gracefully (being single-threaded and all), it causes a delay between stats being emitted to Gymnasticon. If this keeps occurring, it's possible for us to fall behind, and given that it's a very simple check, I figured this was safer.
There was a problem hiding this comment.
Could we remove the receiveTrigger option and just always send the poll requests? It seems that would work for both types of wiring. The main benefit would be UX: the user can just plug-in and ride with any wiring without editing cli/config. We could just default to polling mode, but I'm wondering if there's a use-case for the passive mode anymore?
There was a problem hiding this comment.
I'll need to test a few scenarios from a wiring front -- If I leave the serial Tx floating, it should be safe to always send the poll requests. The real grail for this would be allowing gymnasticon to poll the bike while still allowing for metrics to be received by the tablet.
There was a problem hiding this comment.
Okay -- Tested this with manual rewiring. I can toggle between having gymnasticon be the authoritative source, or the Peloton Tablet, and both are confirmed working with this latest commit. I'm sketching out some connectivity to trace -- I might draw up specs for a small project to give people a hardware toggle switch between the authoritative sources while maintaining stats on the Tablet itself for those interested.
Thanks so much for continuing the work here, I really appreciate it! |
|
@jeremydk thanks for picking this up, excited to add this feature! |
…emydk/peloton_direct_polling
|
Here's a dump of 1s worth of responses. Looking at the second value, we can determine the response type. Looking at the timestamps from the parsing, it does look like you're correct, we're iterating through sending a request every 100ms. I'll get this patched to replicate the behavior. |
|
Traces from the latest patch, metric requests driven from Gymnasticon directly. |
|
Yeah, I think it was a holdover from the initial design. I’ll get a commit
out in a few.
…On Fri, Feb 12, 2021 at 10:45 AM ptx2 ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/bikes/peloton.js
<#43 (comment)>:
> @@ -87,6 +109,21 @@ export class PelotonBikeClient extends EventEmitter {
this.cadence = 0;
this.onStatsUpdate();
}
+
+ pollMeasurementData(port) {
+ setInterval(function() {
+ for(const key of Object.keys(MEASUREMENTS_HEX_ENUM)) {
+ setTimeout(function() {
+ port.write(Buffer.from(MEASUREMENTS_HEX_ENUM[key], 'hex'), function(err) {
+ if (err) {
+ throw new Error(`Error on writing ${key}; ${err.message}`);
+ }
+ })
+ port.drain();
+ }, SERIAL_WRITE_TIMEOUT); // timeout on the serial write + drain
+ }
+ }, POLL_RATE);
+ }
This is looking good. Thanks for taking the time to confirm the timings!
Do you think we need the setTimeout here anymore? The way I'm reading it,
it will offset the port.write calls by 50ms, e.g.
T=0ms interval called
T=50ms port.write called
T=100ms interval called
T=150ms port.write called
And if we remove it and call port.write directly we get:
T=0ms interval called, port.write called
T=100ms interval called, port.write called
The end result is port.write is still called every 100ms. What do you
think?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#43 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADAM6M3OJBU5P5PY5KS2WH3S6VZNXANCNFSM4XKUO2FA>
.
|
Based on the work in #24 (@vodlogic whom I can't add as a co-author, maybe @ptx2 can work some git magic), I've cleaned up the polling logic. This is working on my bike, but does, as we've previously observed, block the communication between the Peloton Tablet and the Bike once the serial Tx is connected.
This still requires a "crossover coupler", but will allow individuals who want to entirely bypass the (or have broken their) Peloton Tablet to get stats off their bike.
Addresses #22