Skip to content

Remove delays #40

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

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions adafruit_pn532/i2c.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ def _wait_ready(self, timeout=1):
with self._i2c:
self._i2c.readinto(status)
except OSError:
self._wakeup()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dont remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that when you issue a command to the PN532 _wait_ready is called before it is ready to respond, which results in an OSError. I tried to check in CP what error was being triggered and it was a NACK. When I removed this line it ends up only taking a few milliseconds to send the ready status. But with this line, it got stuck sleeping for half a second. And if there is no _req pin setup, the only thing that self._wakeup does is sleep for half a second, which is overkill for here.

It is also worth noting that this problem didn't present itself on older versions of CP for me. I think some of the newer performance improvements are the reason this is surfacing. _wait_ready was never called fast enough after a command to cause the OSError to be thrown like it is now. Sooo that's kind of cool lol.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that when you issue a command to the PN532 _wait_ready is called before it is ready to respond, which results in an OSError. I tried to check in CP what error was being triggered and it was a NACK. When I removed this line it ends up only taking a few milliseconds to send the ready status. But with this line, it got stuck sleeping for half a second. And if there is no _req pin setup, the only thing that self._wakeup does is sleep for half a second, which is overkill for here.

Why not return early from _wakeup if _req isn't available? Seems like we do want the ability to sleep and wake the device.

It is also worth noting that this problem didn't present itself on older versions of CP for me. I think some of the newer performance improvements are the reason this is surfacing. _wait_ready was never called fast enough after a command to cause the OSError to be thrown like it is now. Sooo that's kind of cool lol.

🎉 Does the OSError vary depending on whether it's asleep or waking up?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not return early from _wakeup if _req isn't available? Seems like we do want the ability to sleep and wake the device.

That seems like a good idea.

🎉 Does the OSError vary depending on whether it's asleep or waking up?

It doesn't seem to matter. I'm not 100% sure, but when running one of the simple examples each time you loop through it you always get the same OSError.

I've been looking through the PN532 manual, and while I definitely am not familiar with reading this kind of thing, so I could be wrong, but I think the issue doesn't have to do with waking up. At least not the problem I was seeing.

Screen Shot 2020-08-25 at 8 51 00 PM

I think waking up happens in between I2C address Write and COMMAND frame. The OSError is being thrown when we try to read in between COMMAND frame and I2C address Read + Acknowledge frame. So wouldn't calling _wakeup be unnecessary?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know anything about the sensor itself. Does it power down automatically or can we track when the driver shuts it down?

continue
if status == b"\x01":
return True # No longer busy
Expand All @@ -102,8 +101,6 @@ def _read_data(self, count):
i2c.readinto(frame) # ok get the data, plus statusbyte
if self.debug:
print("Reading: ", [hex(i) for i in frame[1:]])
else:
time.sleep(0.1)
return frame[1:] # don't return the status byte

def _write_data(self, framebytes):
Expand Down