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

Include recording settings in binary format #10

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

alexnask
Copy link

A new magic string, b'SDRFFX' is used to specify that we are using the new format.
All arguments passed to the SoapyDevice are captured, as well as the arguments to SoapyPower.sweep()

A device info string from detect_devices is also recorded (which includes all available information, like the label, the serial and the driver of the device).

Test recording:
image

Test read script:

import soapypower.writer

formatter = soapypower.writer.SoapyPowerBinFormat()

with open('output', 'rb') as f:
    header, _, args, device_info = formatter.read(f)

    print("DEVICE INFO:", device_info)
    print("DEVICE HEADER:", args['device'])
    print("SWEEP HEADER:", args['sweep'])

    print("REGULAR HEADER:", header)

Output:
image

Closes #6.

@xloem
Copy link

xloem commented Mar 26, 2018

alexnask, thanks so much for this work!

@xmikos, are you around to merge this PR?

alexnask, if there’s no reply from xmikos, ping me and I’ll close the issue

@xmikos
Copy link
Owner

xmikos commented Mar 26, 2018

Hello, I am still around, but I had no time to work on my SDR projects :-( I will try to look at it and merge it today or tomorrow evening.

@alexnask
Copy link
Author

alexnask commented Mar 26, 2018

@xmikos
Hi, thanks for your time :)

The only part I'm a bit unsure about is

device_info = simplesoapy.detect_devices(self._args['device']['soapy_args'], True)[0]

The assumption is that SoapySDR opens the first device that matches the soapy_args and that the order is the same as the detect_devices function, it seems to work out well with some testing.

Apart from that, the rest of the code is just reading and writing the arguments passed to the SoapyDevice and to the sweep function, it is pretty trivial (the only slight complications are strings, where length + data is written and the time_limit option which can be None).

@alexnask
Copy link
Author

@xloem
Hi, I would really appreciate if you could close the issue if you are satisfied with this solution.
I will be working on any improvements on this pull request if xmikos deems they are necessary, so that it can be merged in master.

@xloem
Copy link

xloem commented Mar 28, 2018

thanks alexnask; I'll close.

I've started using your work already, although I haven't tried reading the data back for processing yet. My only concern is I noticed the header information is output with every single data packet. This might be overkill. What do you think?

@alexnask
Copy link
Author

alexnask commented Mar 28, 2018

@xloem
Yes, this is true.
I could make it a file header instead of a header for every measurement run, I just wrote it that way to match the old binary format, it seems that each packet is independent by design.

@alexnask
Copy link
Author

@xloem @xmikos
Updated the code to write/read the recording settings once, in the beginning of the file.
Here is a test reading script:

import soapypower.writer

formatter = soapypower.writer.SoapyPowerBinFormat()

with open('output', 'rb') as f:
    args, device_info = formatter.read_header(f)
    print("DEVICE INFO:", device_info)
    print("DEVICE HEADER:", args['device'])
    print("SWEEP HEADER:", args['sweep'])

    i = 1
    tup = formatter.read(f)
    while tup is not None:
        tup = formatter.read(f)
        i += 1

    print("READ {} MEASUREMENTS".format(i))

If you need any help converting your measurements from the previous format to this one, I could write a script to do so.

@xloem
Copy link

xloem commented Apr 4, 2018

I've been using this patch locally for a while and it seems to work very well!

If it were to be made perfect, I came up with the following possible concerns:

  • The version field of the old SDRFF header has not been incremented. It appears the intention of this field was to provide future compatibility. It might make sense for the new magic string to be SDRFF with version 3 rather than 2.
  • The settings passed to the device are recorded, but often the driver adjusts these settings. For example, if 2000000 is passed as the sample rate to my rtl-sdr, the actual sample rate will be set to 2048000 or somesuch. For the purpose of processing the data, it would be good to record the actual settings used for the recording.
  • The software version is not recorded. This is important for knowing what set of code produced the file. I plan to push a PR to simplesoapy to allow access to the soapysdr version information.
  • It would be nice to record buffer overflows, which are logged when encountered while streaming, even just with a single count. This information can inform the meaning of the data, as buffer overflows would generate discontinuities if I understand correctly.
  • The overall field format is hard-coded. This means that when additional parameters are added, a new incompatible file format was be implemented. It might be better if the field format were more flexible, such as a string/type/value map or even just an additional field for extra information.

I was thinking, to resolve some of the above, it might be simpler to record simply the commandline arguments as an array of strings, and the resulting device information. I'm not sure, though.

@alexnask
Copy link
Author

alexnask commented Apr 4, 2018

@xloem
Those are some good points.
I will look into them and get back to you.

I don't think passing the command line arguments as an array of string is the best solution but as you say any other solution entails code duplication (forwarding all arguments/settings to the file writer).
I chose to record the arguments the way I did so that a recording environment can be recreated by building a SoapySDR device and calling the sweep function.

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 this pull request may close these issues.

3 participants