Closed GoogleCodeExporter closed 9 years ago
Thank you! When I get a chance, I'll integrate your work.
I'll give you access to the repo at some point soon; I'm just pretty busy at
the moment. I'll let you know when I arrange it.
Original comment by pmalms...@gmail.com
on 1 Oct 2010 at 3:26
No worries, glad to help!
Original comment by gdr...@gmail.com
on 1 Oct 2010 at 3:30
Hi gdrapp,
I took a look at your code today, and I like what you've done. However, I have
a few questions/comments about some of the choices you made.
-- About the new argument api_mode for XBeeBase. Is there a reason why you
chose to make this a numerical argument instead of something like escape=True?
I saw your comment about matching up with the docs for the API-mode integer
specification, so I can see where you're coming from. However, there are only
two choices here (escaped mode or not), so wouldn't a boolean keyword argument
be more self-explanatory?
-- About the placement of your code in XBeeBase. It seems to me that escaping
data is more associated with a raw frame of data (and therefore APIFrame) than
internal frame structure generation and parsing (XBeeBase). I think it would
make more sense if an APIFrame knows whether it is escaped or not and
outputs/parses itself accordingly. For example, this could include an _escape()
method which returns the escaped version of a given byte string. Output() would
conditionally call it if the APIFrame should be escaped.
-- I like how you unescape data on the fly, but perhaps this logic could be
moved into APIFrame as well. For example, we could ditch the current
APIFrame.parse() static method and make an instance method called something
like fill() which would accept a single byte. This method would know whether to
escape incoming bytes or not, and would add them to the APIFrame accordingly.
This would require a few changes to APIFrame's internal layout and how other
parts of the library access the data within an APIFrame, but this is not
unreasonable.
-- Nice unit tests; that's exactly what I'm looking for.
I look forward to seeing your thoughts.
Original comment by pmalms...@gmail.com
on 12 Oct 2010 at 7:51
My thought on the api_mode argument was that by using an integer, if Digi
decided to add a third API mode it would be easier to implement without another
API change in the XBeeBase class.
I like your ideas for moving the escaping code into APIFrame. I originally
thought about doing that but read somewhere that escaping should be the first
thing you do when you receive a byte and the last thing you do before you send
one, so I put the code as close to the sending/receiving and I could.
Overall, I'd don't have any issues with your proposed changes. If I get some
time, I'll see what I can do. If you decided to take it on, please let me know
so we don't duplicate work.
Thanks.
-Greg
Original comment by gdr...@gmail.com
on 13 Oct 2010 at 1:14
Hi Gregg,
I found some time this evening to work on the code. It's pretty close to being
fully integrated; I just need to add a few more tests. I'll have a new release
cut hopefully by the end of the week with your code in it.
Thanks,
~Paul Malmsten
Original comment by pmalms...@gmail.com
on 15 Nov 2010 at 2:59
Fixed as of version 2.0.0.
Original comment by pmalms...@gmail.com
on 29 Dec 2010 at 10:03
Original issue reported on code.google.com by
gdr...@gmail.com
on 19 Sep 2010 at 1:48Attachments: