CWL's click to move is unreasonably slow

[eoinell]

~5s to respond. The actual frame rate is good though.

[eoinell]

Newest version of open_cv doesn't alleviate this

[eoinell]

The issue can be traced back to MessageBusInstrument's https://github.com/nanophotonics/nplab/blob/6a191088739e35d7ba8ab28fa38327ebcdb7ea67/nplab/instrument/message_bus_instrument.py#L149 This is what's adding the ~4s to the move time. It doesn't help that Stage's get_position is called thrice in the CWL's move_to_pixel.

All this can be gotten around by storing the _last_position of the stage locally, however currently we would have no way of knowing when the position is changed by the joystick.

[eoinell]

It works super fast in python 2 on the python-3 branch however. Very curious to know what's causing this

[eoinell]

https://github.com/nanophotonics/nplab/blob/78ba3dd27b931a5b652e7440f404c7ee675a90d4/nplab/instrument/serial_instrument.py#L111 this line takes exactly 1 second

[eoinell]

https://github.com/nanophotonics/nplab/blob/e7cb7be7f24ad779554e4adf4cc710b0180e0a3e/nplab/instrument/stage/prior.py#L22 presumable because of this timeout.

So it's not recognising the end-of-line character? https://github.com/nanophotonics/nplab/blob/e7cb7be7f24ad779554e4adf4cc710b0180e0a3e/nplab/instrument/stage/prior.py#L26 Or is it expecting a certain number of bytes and waiting?

I could 'fix' this by setting the timeout to 0.1s, but it does indicate that something's not quite right here. @YagoDel @wdeacon @rwb27 any thoughts?

[YagoDel]

Full disclosure, I don't use this section of nplab.

That being said, does changing the timeout to 0.1s actually fix the "unreasonably slow" response? If so, then my intuition is also that there is an issue with the serial messaging. But since I'm also using messaging instruments and they are working fine, it must be a bug in the prior.py I'd recommend printing out all reads and queries and comparing between Python 2 and 3

[eoinell]

https://stackoverflow.com/questions/16470903/pyserial-2-6-specify-end-of-line-in-readline Implemented this fix. I think this thread is where the original version of readline comes from anyway.

[YagoDel]

Well that solution isn't ideal. An obvious thing is that it doesn't acquire the communications_lock. A less obvious one (and I'm not sure if there's a solution) is that it feels like it's doing something the PySerial package is already doing underneath. Other answers in that StackOverflow seem neater?

[eoinell]

is the communications_lock issue simply a matter of adding with communications_lock:? Yeah, it seems that ser.read_until(self.termination_character) should work

[eoinell]

with self.communications_lock:
            return self.ser.read_until(self.termination_character).decode()

I just tried this, which again times out.

[YagoDel]

So the only communication issue is with readline?

[eoinell]

Yeah. I think it only becomes an issue when using a non-standard EOL as well.

[rwb27]

@eoinell the link on stackexchange looks very much like what I did to work around pyserial's lack of support for non-newline end of line characters. That code certainly broke for us in Python 3, but I have a nasty feeling our solution was to get the Arduino to use \n rather than enabling Python to parse \r.

I think setting timeout to 0.1s is a bit nasty, it looks like your fix in 725ce5b does it "properly" though it's frustrating that you have to write this; I feel like this definitely ought to be a standard library function. It definitely used to work without timeouts in Python 2, using the IO wrapper, but if you have a solution that works properly, it's probably not worth burning more time chasing this down.

[eoinell]

I absolutely agree that 0.1s wasn't great and was a quick fix while I investigated further. The current code doesn't time out however happily, even if it's not the most compact.

---

From: Richard Bowman notifications@github.com Sent: Monday 27 January 2020, 16:54 To: nanophotonics/nplab Cc: Eoin Elliott; Mention Subject: Re: [nanophotonics/nplab] CWL's click to move is unreasonably slow (#93)

@eoinellhttps://github.com/eoinell the link on stackexchange looks very much like what I did to work around pyserial's lack of support for non-newline end of line characters. That code certainly broke for us in Python 3, but I have a nasty feeling our solution was to get the Arduino to use \n rather than enabling Python to parse \r.

I think setting timeout to 0.1s is a bit nasty, it looks like your fix in 725ce5bhttps://github.com/nanophotonics/nplab/commit/725ce5b13e52f8550fc11b7297d7c405cba770e0 does it "properly" though it's frustrating that you have to write this; I feel like this definitely ought to be a standard library function. It definitely used to work without timeouts in Python 2, using the IO wrapper, but if you have a solution that works properly, it's probably not worth burning more time chasing this down.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/nanophotonics/nplab/issues/93?email_source=notifications&email_token=AKORCB2WBFRRTHI7LBOADBLQ74GV5A5CNFSM4JW5WZ72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKAHJII#issuecomment-578843809, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AKORCB37ODXSLEJ5XV55OETQ74GV5ANCNFSM4JW5WZ7Q.

[rwb27]

yeah - the current code looks like it does exactly what the bufferedIOWrapper ought to be doing. Glad you got it working sands timeout though :)