niolabs / python-xbee

Python tools for working with XBee radios
MIT License
101 stars 45 forks source link

xbee.wait_read_frame() timeout issue #59

Closed kenanelgaouny closed 6 years ago

kenanelgaouny commented 6 years ago

Hello, it seems that xbee.wait_read_frame() is not timing out when a timeout is specified Here's what i have tried:

data = xbee.wait_read_frame(1)

Traceback (most recent call last): File "<pyshell#32>", line 1, in data = xbee2.wait_read_frame(1) TypeError: wait_read_frame() takes exactly 1 argument (2 given)

ser = serial.Serial('COM5', 9600, timeout=1) xbee=DigiMesh(ser) data = xbee.wait_read_frame() ##stuck here until a packet is recieved

any help would be appreciated !

jamesleesaunders commented 6 years ago

Is this the same issue described in #49 ?

kenanelgaouny commented 6 years ago

Yes they are related, but Issue 49 talks about the case where an incomplete frame is received. For this issue the timeout is for when no data is received at all

jamesleesaunders commented 6 years ago

One of the most common reasons for unpredicted frames is when the Xbee API settings are not correct.

What AP and AO settings have you got?

Try: API Enable (AP): 2 API Output Mode (AO): 3

Depending on what the above is you may also need to play with the ‘escaped=true’ setting when when constructing in Python.

ser = serial.Serial('/dev/ttyUSB0', 9600) xbee = ZigBee(ser, escaped=True)

Also see: https://github.com/niolabs/python-xbee/issues/26

jamesleesaunders commented 6 years ago

Did you find the solution/fix?

kenanelgaouny commented 6 years ago

yeah it was more of a workaround, i used another older version of the library that can be found here: https://github.com/thom-nic/python-xbee, with this library the timeout works fine and the line data = xbee.wait_read_frame(1) doesnt not cause an error.

jamesleesaunders commented 6 years ago

Ah, interesting, we should probably work out what the difference is in the @thon-nic version and get the fix encorporated back upstream.

XYR010 commented 6 years ago

Maybe something along the lines of this, i looked into the change history of xbee/thread/base.py and its predecessor: xbee/base.py, in neither was the version of _wait_for_frame with self and timeout=None as args, so probably it wasn't in there since the beginning

diff --git a/xbee/thread/base.py b/xbee/thread/base.py
index 3b60c8b..b47ec45 100644
--- a/xbee/thread/base.py
+++ b/xbee/thread/base.py
@@ -105,7 +105,7 @@ class XBeeBase(_XBeeBase):
         frame = self._wait_for_frame()
         return self._split_response(frame.data)

-    def _wait_for_frame(self):
+    def _wait_for_frame(self, timeout=None):
         """
         _wait_for_frame: None -> binary data

@@ -119,6 +119,10 @@ class XBeeBase(_XBeeBase):
         """
         frame = APIFrame(escaped=self._escaped)

+        deadline = 0
+        if timeout is not None and timeout > 0:
+            deadline = time.time() + timeout
+        
         while True:
                 if self._callback and not self._thread_continue:
                     raise ThreadQuitException
@@ -130,6 +134,8 @@ class XBeeBase(_XBeeBase):
                 byte = self.serial.read()

                 if byte != APIFrame.START_BYTE:
+                    if deadline and time.time() > deadline:
+                        raise TimeoutException
                     continue

                 # Save all following bytes, if they are not empty
bp1222 commented 6 years ago

So after reading the PR & thon-nic addition it sure seems as though this is a feature request. From what I can tell, this implementation was designed to be used in one of two ways. 1) as a blocking call, where calls to wait_for_frame would block and return a frame when available. and 2) as an async callback, where the wait_for_frame would run in it's own thread, hitting some application defined callback when data is available. Assuming you'd be putting that into some threadsafe queue that could then be processed in your applications main loop.

That said, I don't necessarily have an issue with this fix. My only complaint would be that the threading mode, and tornado implementations would have different function signatures. We'd probably want to update the tornado side of this as well to properly raise the exception if a frame is not received within the timeout window.

jamesleesaunders commented 6 years ago

@bp1222 you make an excellent point about the signatures! Do you (or @xyr010) fancy making this change to the tornado implementation also?

bp1222 commented 6 years ago

@jamesleesaunders I can next week sometime, unless @XYR010 if you're familiar/interested in applying this feature to the tornado side.

XYR010 commented 6 years ago

I didn't work on the tornado version yet, but i will try. when reading through thom-nics version i noticed that he uses a new timeout once the start-byte was read, so that incomplete frames timeout, maybe something like this should be added as well later on

2018-03-15 21:48 GMT+01:00 David Walker notifications@github.com:

@jamesleesaunders https://github.com/jamesleesaunders I can next week sometime, unless @XYR010 https://github.com/xyr010 if you're familiar/interested in applying this feature to the tornado side.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/niolabs/python-xbee/issues/59#issuecomment-373517829, or mute the thread https://github.com/notifications/unsubscribe-auth/AGdE9nhqLvzHkcMUjebMXSIQWgzUMDz5ks5tetOEgaJpZM4QIlj3 .

bp1222 commented 6 years ago

This issue has been merged into master.