pySerialTransfer: Reading is slow
Hey there, I firstly want to say that this library (and its Arduino companion) is awesome and makes communicating with Arduinos so much easier.
I’ve been running into some issues reading data from multiple Arduinos at the same time. If I just read from one Arduino, it only takes 1-2ms for transfer.available() to complete (measured using time.perf_counter). But if I connect 2 devices that are both sending data to the computer at 10hz, the readings sometimes only take 1-2ms, but sometimes they take 20-70ms. Usually it starts at 1-2ms for a few seconds, then it starts taking 20-70ms for the rest of the time it is running.
The 2 transfer objects are on separate threads, so I imagine this is the root of the issue somehow… I am sending data on a separate thread, but I tried adding locks and that didn’t fix the problem.
If I never send any data and just receive, then it always runs at 1-2ms. If I send data for a few seconds and then stop sending data, then it runs at 20-70ms, even well after I stopped sending data.
Each transfer receives in its own thread in this method:
def run_serial_receiver(self, remote: transfer.SerialTransfer, lock):
while True:
time.sleep(.001)
with lock:
t0 = time.perf_counter()
if remote.available():
dt = (time.perf_counter() - t0)
print(dt)
if(dt>.01):
print(f"EXCESSIVE TIME: {dt}")
sliced_buff = remote.rxBuff[0:remote.bytesRead]
rxbytes = bytes(sliced_buff)
self.process_RemoteMessage(rxbytes)
And data is sent using this method on the main thread:
def send_output(self, output):
packet_size = len(output)
for i, remote in enumerate(self.remotes):
with self.serial_locks[i]:
remote.txBuff = output
remote.send(packet_size)
if self.send_callback:
self.send_callback()
I’m running on Windows 10, with the SerialTransfer library running on a Teensy 4.0.
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 22 (22 by maintainers)
Commits related to this issue
- Fix latency issue mentioned in #36 — committed to PowerBroker2/pySerialTransfer by PowerBroker2 3 years ago
- Revert "Fix latency issue mentioned in #36" This reverts commit b19168115e088e06a69d52875af4d4c90d7cfab7. — committed to kyleb-imp/pySerialTransfer by kyleb-imp 3 years ago
- Fix latency issue mentioned in PowerBroker2#36 by removing repeated calls to self.connection.read(). On Windows (and maybe other OSes), the PySerial read() function can take approx 1ms, even when only... — committed to kyleb-imp/pySerialTransfer by kyleb-imp 3 years ago
@PowerBroker2 Sure thing, I’ll push my patch through as a PR shortly.
@kyleb-imp Thanks for your thoughts. I didn’t explain this in my previous comment so should now mention it! What I found during some testing is that there is still a bit of a problem with the new code, because the previous code was doing byte-by-byte indexing on
self.rxBuff
which was always pre-allocated to a length ofMAX_PACKET_SIZE
and would always stay at this length. With the new code, I think this line here:https://github.com/PowerBroker2/pySerialTransfer/blob/4cd4e772447d4e14c62a0bcddbc5199bcae15e9d/pySerialTransfer/pySerialTransfer.py#L536
… will now end up truncating the
self.rxBuff
to a length smaller thanself.bytesToRec
in the situation where there is an incomplete read from the OS due to a timeout e.g. if the library user sets a timeout on PySerial. Then, this line was giving me an index out of range exception ascrc.calculate
expects there to be at leastself.bytesToRec
in theself.rxBuff
:https://github.com/PowerBroker2/pySerialTransfer/blob/4cd4e772447d4e14c62a0bcddbc5199bcae15e9d/pySerialTransfer/pySerialTransfer.py#L540
… hence the patch which changes things around to maintain
self.rxBuff
at the pre-allocated length like it used to be.@kyleb-imp This change unfortunately creates a bit of a problem, which is, a call to
link.tick()
can now block in the OS whereas before it should never block (because, if I understand things right, the library would previously only ask for bytes from pySerial if pySerial actually had bytes available to give out). This means that existing code that has awhile True
loop of some sort which is doing other stuff in addition to.tick()
in the loop now might be blocked by the OS from being able to do its other stuff (e.g. consider the situation if there is an error condition on the link and a truncated packet is sent out). However, I wholly agree that asking the OS for as many bytes as we expect that we should receive is the best approach, this also burns far less CPU cycles overall. So, I think adding a timeout to pySerial’s configuration might be a way forward here. Then,link.tick()
should not block forever if there is insufficient data received on the link due to an error condition. Could you please try out this patch? pyserialtransfer-timeout-patch.txtFixed in 2.1.3