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

Address several issues discovered in testing #18

Merged
merged 4 commits into from
Sep 8, 2021
Merged

Conversation

sjkelly
Copy link
Collaborator

@sjkelly sjkelly commented Sep 7, 2021

  • Partially addresses Large StreamBuffer requests do not block until fulfilled #17 by removing the assert in read, and allowing fewer samples as allowed by the semantics of Julia's read

  • Simplifies the SoapySDR <-> Julia Type pathway. We eliminate the StreamFormat container, and just use a few helper functions to keep this abstraction from leaking through the high level API.

  • Constructing a new Stream will use the native device format by default (we have multiple dispatch, so we should use it.). It is also now aware of the MTU, thus giving a simplified read function without a specified length.

I think this should encapsulate the API changes to address the issues discovered in testing (thanks @mbaz !) I'll need to add some more docs here to full clarify how the Stream interface works, mainly regarding activate!/deactivate! functions if not using continuous processing. (I'm investigating if we have IO interface semantics in Base for this already)

re: #17 #13 #12 #8

src/highlevel.jl Outdated Show resolved Hide resolved
This should be slightly more robust in that we have explicit type mappings.
We also add a simple promote mechanism to use the native type if unspecified
in the Stream call.
@sjkelly
Copy link
Collaborator Author

sjkelly commented Sep 8, 2021

I put back the old assertion, with an additional comment in the warning. The main issue can be avoided by read(...;all=false) to allow fewer samples than requested. We will need to block and loop until we fill the requested read amount. May be good to warn if not aligned on the MTU as well. I will track in #17

@sjkelly sjkelly merged commit 37a832e into main Sep 8, 2021
@sjkelly sjkelly deleted the sjk/highlevel3 branch September 8, 2021 04:18
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.

None yet

2 participants