-
Notifications
You must be signed in to change notification settings - Fork 76
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
first implementation of wav2sigmf.py #216
Conversation
I'm ok with this PR taking a while to percolate, but I kinda would appreciate hearing whether there is general interest in the project accepting and maintain a battery of converter scripts/functions to facilitate coming over to the SigMF side. |
I have thought about this a few times, and am leaning against having converters like this within this repository primarily because of the natural tendency for tools like this to become an unbounded source of maintenance. E.g.: "we have wav, lets add support for flac... pcm... mp3.." and starts to pull us away from the original mission. Id like to hear the opinions of other maintainers though @bhilburn @777arc @Teque5. |
I would take what we can get as long as it doesn't messy up the rest of the code or cause increased maintenance in the long term, e.g., if we anticipate having to update this wav2sigmf.py in the future as part of changes to the standard, but I don't think that will be the case because backwards compatibility is pretty critical with this kind of standard. If someone else comes along and creates flac/pcm/mp3 converters, that's great, as long as it doesn't use a ton of the rest of the maintainer's time. I think if anything the discussion should be more about whether or not we want a "converters" dir in the root =P |
Yea I think this kind of tool is pretty useful. If it uses the intended entry points for If we decide to accept this PR, I can modify it to add an entry point in the |
Maybe @gmabey will give you write access to his fork to make those additions 😃 |
Ok there is sufficient support and I my initial position was very weak. Let's get these changes made and bring it in. In terms of code structure, I think it might be better to have this in an "examples" dir as opposed to "converters". Thoughts? |
I think I still like When I hear the word "examples" I think it implies in my mind that it doesn't actually do anything useful. |
I'm happy to accept and review PR's to my fork |
Examples that are not useful will get the 🧹 I am suggesting a more broad directory as the SigMF project is invested in building more examples and this is a god one. It also takes the pressure and expectation away from the effort providing comprehensive conversion tools. @777arc you had a similar comment above thoughs? |
It's also possible this belongs in @bhilburn should we require similar DCO signoff as GR does? This is a significant contribution. |
In my mind, there is a significant difference between "sample code" (to draw a C++ analogy, code that demonstrates how to properly invoke a function) and a "utility" (like, a compiled application that performs a specific operation and does it reliably). In the spirit of encouraging others to adopt SigMF as a file format (and leave behind others), I suggest we aim towards the latter.
Hmm, you want to avoid the impression that |
Utils or tools are also highly reasonable. At the end of the day this is a fairly specific tool but it has the widespread benefit of showing how the module can be used in a simple and straightforward way. My goal is to ensure the project can retain adequate focus on the development of the specification. There are myriad converters we could include and should not. Our tooling already has issues with bitrot as you have identified. Thinking more, this definitely needs go inside |
I like /examples or /utils or /tools over /converters because there will be non-converter things we add and we aren't trying to collect an enormous list of converters. Yeah definitely inside sigmf/ and at some point the python module may get put into its own repo anyway and this might as well go along with it. |
I did move I think that this file is unique (in SigMF) in that it's intended to be invoked as a command-line application, but is also useful for |
LGTM |
In order to see what works and what doesn't, I suggest you run this: