Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible I2C speed up for digital sensors #11

Open
GoogleCodeExporter opened this issue Oct 10, 2015 · 13 comments
Open

Possible I2C speed up for digital sensors #11

GoogleCodeExporter opened this issue Oct 10, 2015 · 13 comments

Comments

@GoogleCodeExporter
Copy link

Hey guys,

I've just been reading through the sensor.py code (current trunk). I might've 
spotted a way to reduce the number of direct commands necessary to retrieve a 
sample from a digital sensor. In Bluetooth connections (which are fairly slow 
as you probably know), this can sometimes/often save one out of 3 direct 
commands and reduce the time to query a digital sensor by roughly 30ms.

I'm not sure if I understood the whole nxt-python package correctly, but I 
assume that all sample retrieval communication with digital sensors is 
happening through the method "i2c_query()" from the class DigitalSensor (here: 
http://code.google.com/p/nxt-python/source/browse/trunk/nxt/sensor.py#128 ). If 
that's correct, then in line 135, _ls_get_status() get's called, which seems to 
do busy waiting until the I2C bus / the sensor has finished its current 
operation. Only when enough bytes are ready (or in case of a timeout), the 
polling (direct command LSGetStatus) will stop.

Now the point: If LSGetStatus tells you "everything ok, data ready to be 
retrieved", you have to issue the direct command LSRead. This is where you can 
save time. You can totally ignore the LSGetStatus commands, and allways try 
LSRead straight away. As long as there isn't enough data, you can detect this 
situation from the status byte of the reply package. But if you're lucky, i.e. 
once the sensor is ready, you already got the valid data as payload. In summary 
this saves 1 command.

We've been using this method for a long time successfully, and it did greatly 
improve the performance with all digital sensors (the US being most widely 
used).

An implementation example with further explanations and some statusbyte-codes 
can be found here:
http://www.mindstorms.rwth-aachen.de/trac/browser/tags/version-4.03/RWTHMindstor
msNXT/COM_ReadI2C.m#L139


I'd love to see you guys use that principle and see if you can improve your 
Bluetooth performance. If not that's fine, but you could keep this in mind and 
maybe on the to-do-list.
Thanks for reading.

Regards, Linus

Original issue reported on code.google.com by [email protected] on 19 Aug 2010 at 12:36

@GoogleCodeExporter
Copy link
Author

Excellent idea, and nice to see you again Linus. I spent an hour or two writing 
a quick bit of code to implement this, and it's looking good. When using 
bluetooth, the time for one sample goes from 162ms to 112ms, a reduction of 
31%. When using USB it's 45 to 33 or 26%. With some intense hacking I was able 
to further reduce it to 28ms by sending a another ls_write request directly 
after sampling to prime for the next read, but that method carries the risk of 
a small nuclear explosion.

Actually, it just messes things up if you're not trying to probe at insane 
speeds.

Anyway, this is promising. I will commit my changes and see if anyone finds 
problems with them. For now, fixed. I may also look into this in v2.

Original comment by [email protected] on 20 Aug 2010 at 1:43

  • Changed state: Fixed
  • Added labels: Series-All

@GoogleCodeExporter
Copy link
Author

In v2, the exact same changes can be made: For bluetooth, the inprovement is 
from 145ms to 100ms, or 31% decrease. With USB, it's 45ms to 31.8ms, 29%.

Thanks for pointing this out!

Original comment by [email protected] on 20 Aug 2010 at 3:28

@GoogleCodeExporter
Copy link
Author

Unfortunately, this caused a bug with query results detailed in Issue 13, so I 
had to revert it. What a shame!

Original comment by [email protected] on 5 Sep 2010 at 10:02

  • Changed state: WontFix

@GoogleCodeExporter
Copy link
Author

I have noticed another possible bug in our code which may have been causing 
Issue 13 and will be taking another look at this. See my comment to that ticket 
for more details.

Original comment by [email protected] on 6 Feb 2011 at 4:45

  • Changed state: Started

@GoogleCodeExporter
Copy link
Author

I have no idea what happened to my effort on this and can only guess that I 
couldn't get it to work at all. In any case I'm not working on it at the moment.

Original comment by [email protected] on 12 Apr 2011 at 5:02

  • Changed state: WontFix

@GoogleCodeExporter
Copy link
Author

Marcus, is it possible for you to continue work on this? Those speed increases 
would be very useful if it works correctly.

Original comment by [email protected] on 10 May 2011 at 2:36

  • Changed state: Questionable

@GoogleCodeExporter
Copy link
Author

Everything I tried introduced some new bug with the communication. I tried many 
fixes and none of them helped. If anyone else wants to take a look at this, 
they're welcome to, but I can't make it work.

Original comment by [email protected] on 10 May 2011 at 7:02

  • Added labels: Series-2.x
  • Removed labels: Series-All

@GoogleCodeExporter
Copy link
Author

It's really a shame that this doesn't work. Unfortunately, I'm not familiar 
with the nxt-python sourcecode, and I haven't set it up right now (also I don't 
have an NXT at hand atm). But I can again confirm that this "shorter command 
sequence" works, and we've not yet found a sensor that doesn't support it. I 
can definitely say it works for the LEGO Ultrasonic, and for the HiTechnic 
Accelerometer, Color V1 + V2, IR Seeker, and EOPD sensors.

The whole magic, complete with documentation of the trick, happens in this 
function: 
http://www.mindstorms.rwth-aachen.de/trac/browser/tags/version-4.04/RWTHMindstor
msNXT/COM_ReadI2C.m (I already posted it).

Maybe it helps if I try to describe the whole process, how we do it 
sucessfully, in my own words again:

Open sensor:
1. Set correct sensor mode: Use SetInputMode with LOWSPEED_9V and RAWMODE
2. Let the sensor boot up (depends on sensor, wait between 50 and 300ms).
3. Depending on sensor: Send commands via LSWrite. I.e. for ultrasonic: Send 
CONTINUOUS_MEASUREMENT (0x02) to register 0x41 and address 0x02 (this might be 
enabled by default anyway).
4. "Flush out old data?", a comment made by Dick Swan, developer of RobotC for 
NXT: This is just sending an LSRead command (and collecting the answer packet).

Now the actual retrieving data:
1. Send LSWrite with the register you need to retrieve. You don't neet to 
request an answer packet.
2. Now, in a loop, we keep on polling LSRead
3. a) Statusbyte == 0 or bytesRead > 1 means SUCCESS! Break from loop, 
interpret data
   b) Optional: Statusbyte == 221 or == 224 means errors we probably can't recover from (wrong sensor mode, cable disconnected). Retry or exit, whatever you want, try for yourself.
   c) If timed out: Give up
4. Interpret content of the LSRead command from step 2.


As I said, it's actually very simple. Instead of using LSGetStatus, EVER, you 
can just use LSRead. LSRead returns the statusbyte just as LSGetStatus, but 
ALSO returns data payload if available. So if statusbyte == 0, use the data.


We don't do sensor-name-checking like Vendor ID and so on, but I tried this, 
and it works, too. But maybe this is not the best test case, I don't know.


Anyway, just wanted to tell you guys this and "not let you down". I know it's a 
bit frustrating when people just tell other people what to do and don't 
implement it themselves... Sorry, and good luck anyway.

Original comment by [email protected] on 16 May 2011 at 9:41

@GoogleCodeExporter
Copy link
Author

So actually, Marcus, I know you tried lots of thing and we also had a 
discussion about this in another issue (with a new bug introduced). However, I 
wanted to try to post the "packet sequence" again. It might be that the bug is 
introduced "at a higher level" somewhere in nxt-python. But if looked at the 
whole thing at low "packets in, packets out" debug level, I honestly can't 
imagine why it would in one programming language and not in the other.

As for "drivers": We've tested this on Windows and Linux with both USB and 
Bluetooth (Fantom, serial port, libusb, /dev/rfcomm), and apparently people are 
also using it on Mac OS...

Best regards, Linus

Original comment by [email protected] on 16 May 2011 at 9:46

@GoogleCodeExporter
Copy link
Author

All right, I will look at this again.

Original comment by [email protected] on 17 May 2011 at 1:41

  • Changed state: Accepted

@GoogleCodeExporter
Copy link
Author

I was discussing this issue with the developer of MindSqualls (a package 
similar to nxt-python, but for .net) on the Mindboards forums here: 
https://sourceforge.net/apps/phpbb/mindboards/viewtopic.php?f=3&t=1014

His version of the algorithm is:
1. Use LsWrite() to tell the sensor which I2C register you need
2. Pause 22 ms.
3. Call LsRead() to get the value.
3a. If error == PendingCommunicationTransactionInProgress repeat from step 2.
3b. If error == CommunicationBusError repeat from step 1.
3c. If error == unknown Fail.
4. Return the value from LsRead.


He reports these performance improvements:

> Response-time: 104 ms -> 81 ms
> Commands: 3.9 -> 2.1

Original comment by [email protected] on 17 Aug 2011 at 12:34

@GoogleCodeExporter
Copy link
Author

Some news again from the mindboards thread I linke to above: Apparenty the 
problem that is described in Issue 13 only occurs with USB connections (it also 
says so in Issue 13). This I2C speed-up however is only important for Bluetooth 
connections.

So why not enable this faster algorithm for Bluetooth, but nor for USB?

Original comment by [email protected] on 6 Sep 2011 at 9:01

@GoogleCodeExporter
Copy link
Author

Every time I try to work on this, I break things. If you could come up with a 
patch that works and implements the improvements, it would really help this 
along.

Original comment by [email protected] on 17 Sep 2011 at 5:11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants