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

add b before data string #846

Merged
merged 1 commit into from
Feb 21, 2022
Merged

add b before data string #846

merged 1 commit into from
Feb 21, 2022

Conversation

andynoack
Copy link
Collaborator

Fix #845

@andynoack andynoack added the bug label Mar 5, 2021
@andynoack andynoack self-assigned this Mar 5, 2021
@jopohl
Copy link
Owner

jopohl commented Mar 5, 2021

Nice fix, just two questions about this:

  1. Can we be sure that this will not break the rfcat plugin for users with an old, i.e., the python2 version of rfcat?
  2. Will this fix cause problems on our end once/if RFxmit(...) now successfully accepts strings with python3 atlas0fd00m/rfcat#87 gets merged?

@andynoack
Copy link
Collaborator Author

andynoack commented Mar 5, 2021

Good point, we cannot be sure this does not break anything. Putting the information as bytes with (b'...') should not make much difference, however, even for the old interface. What do you suggest?

@jopohl
Copy link
Owner

jopohl commented Mar 5, 2021

I cannot test this right now, but in worst-case we might transmit an additional "b" for the old version.

Normally, I would suggest to make a simple if-construct and check for the version of rfcat. Unfortunately, the versioning seems to have only one real release: https://github.com/atlas0fd00m/rfcat/releases/tag/v1.0.0

Another pattern would be to use the "ask for excuse instead of permission" approach and do something like:

self.prefix = None
....

# determine the prefix on first call
if self.prefix is None:
  try:
    "d.RFxmit({})".format(data)   # use the old version
    self.prefix = ""
  except TypeError:
   "d.RFxmit(b{})".format(data)   # use the new version
   self.prefix = "b"
else:
  "d.RFxmit({}{})".format(self.prefix, data) 

@pickeditmate
Copy link

rfcat is running on python 3 now anyway, i've not got python 2.7 even installed and rfcat is running smoothly as atlas has made the latest release work and is asking for feedback.

@jopohl
Copy link
Owner

jopohl commented Apr 19, 2021

If rfcat is Python3 now, we might simplify the RfCat plugin a lot by just importing rfcat and calling its methods directly instead of wrapping the rfcat executable.

@andynoack
Copy link
Collaborator Author

I am totally okay with just adding the 'b', but I don't think it is worth the effort to refactor the plugin at the moment. Setting up software and hardware for testing is what currently prevents me from elaborating a nice and clean solution, having in mind that only very few people will take notice.

@jopohl
Copy link
Owner

jopohl commented Apr 19, 2021

That is a good point, this is not core URH. Could someone from the community please test whether this branch works

  1. with the old python2 version of rfcat
  2. with the new python3 version @pickeditmate mentioned

If we can confirm that both cases are functional I am fine merging it as is.

@pickeditmate
Copy link

I tried the rfcat fix branch and also tried, before that, to just add the b to the string, both with no success, I'm begining to wonder if I'm missing something else on my system following those tests.

Could someone confirm what is meant to be seen after pressing the button as I get no light as I usually do when transmitting on the yardstick.

My environment is as follows:
Windows 10 x64
Python 3.9
rfcat latest version (python 3 compatible)

@jopohl jopohl merged commit 4e29d70 into master Feb 21, 2022
@jopohl jopohl deleted the rfcat_fix branch February 21, 2022 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rfcat not sending
4 participants