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

Change from chr to bytes broken code #44

Closed
coylen opened this issue Mar 26, 2019 · 11 comments
Closed

Change from chr to bytes broken code #44

coylen opened this issue Mar 26, 2019 · 11 comments

Comments

@coylen
Copy link

coylen commented Mar 26, 2019

I was trying and failing to change parameters on a new Yardstick one that I had just got. However, it would not change some of the settings. I have reflashed the stick to make sure that that is not the issue and after further investigation, it would seem that the setRFRegisters does not seem to like the change from chr to bytes.

The output of these two functions changes subtly from '\x??' to b'\x??' and the register selected won't change with the second version. I have been able to successfully change the data rate and other parameters by manually working out the values and poking them directly using the first version as input.

@Loumiakas
Copy link

Loumiakas commented Apr 2, 2019

Yes, I have experienced same problem and came to the same conclusion as you, it seems like one of the recent commits broke it.

@atlas0fd00m
Copy link
Owner

@roaldnefs this wouldn't have anything to do with the latest migration to support py3, would it?
could you please verify correct functionality in py2.7+?

thanks
@

@coylen
Copy link
Author

coylen commented Apr 2, 2019

I was using python 2.7 when i had the problem indicated above

edit: sorry didn't notice comment above was directed at someone else

roaldnefs added a commit to roaldnefs/rfcat that referenced this issue Apr 2, 2019
Revert `bytes([])` to `chr()` because it breaks the `d.setRFRegister()` function.

This commit fixes atlas0fd00m#44.

Signed-off-by: Roald Nefs <info@roaldnefs.com>
@roaldnefs
Copy link
Contributor

PR #40 is indeed causing the problems. I've reverted changes in #46. Unfortunately, I do not have time to resolve #44 and maintain compatibility with Python 3.

@coylen @Loumiakas, my apologies for any inconvenience this may have caused...

@atlas0fd00m
Copy link
Owner

@roaldnefs has been putting in significant effort to push RfCat into the Py3 world. this is an oversight on both our parts, please accept both of our apologies. thank you for bringing it to our attention!

@roaldnefs what's the best path forward?

@coylen would you be willing to submit a very simple unittest PR? i've just created a basic unittest file you can add it to: tests/test_basics.py
all you'd need to do is create a new function/method for the RfCatBasicTests class, which imports rflib and asserts some test of behavior. i created a simple example in the file if you're not familiar with unittests.

@Loumiakas
Copy link

Hey @roaldnefs and @atlas0fd00m, I would be willing to volunteer myself to continue where @roaldnefs left off with regards to porting rfcat to Python 3 (if he doesn't mind, of coarse). Granted, it would take some time for me to get up to speed with regards to 'futurize' package and rfcat code base.

@atlas0fd00m
Copy link
Owner

atlas0fd00m commented Apr 3, 2019

@roaldnefs @Loumiakas @coylen
please check out my latest commit. i took a slightly different tack on @roaldnefs 's backout PR. let me know if everyone is happy.

@roaldnefs
Copy link
Contributor

roaldnefs commented Apr 3, 2019

@Loumiakas feel free to continue where I left off! If you create a work in progress PR I might be able to help you if I can find some free time.

@atlas0fd00m, if I'm not mistaken your solution still creates the same problem (in Python 3). If you attempt to run bytes([value]) over a bytes value this causes a TypeError, e.g.:

# Python 3.6.7 
>>> from builtins import bytes
>>> value = bytes([1])
>>> value = bytes([value])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'bytes' object cannot be interpreted as an integer

A possible solution might be to check for the value type in correctbytes().

@atlas0fd00m
Copy link
Owner

i'm not sure what you're referring to, @roaldnefs , do you have an example with more context? what i'm seeing looks like you're creating a "bytes" object, then handing that into the list constructor []. that isn't what correctbytes() is supposed to do.
what i committed works pretty well on py2.7 that i'm working on.

@roaldnefs
Copy link
Contributor

roaldnefs commented Apr 3, 2019

@atlas0fd00m, you solution should work fine in Python 2.7, but there are still problems when using Python 3 caused by line 23 in rflib/bits.py. I've updated my previous comment for clarification.

@atlas0fd00m
Copy link
Owner

dammit. i promised myself not to miss those.
i left [] around a couple of them. try now, @roaldnefs

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

Successfully merging a pull request may close this issue.

4 participants