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 new Symbol Synchronizer blocks #1294

Closed
wants to merge 16 commits into from

Conversation

awalls-cx18
Copy link
Contributor

@awalls-cx18 awalls-cx18 commented Apr 23, 2017

Add new symbol synchronizer blocks to gr-digital and their required supporting classes to gr-digital and gr-filter.

These blocks are a superset of the Clock Recovery M&M blocks, MSK Timing Recovery Block, and PFB Clock Synch blocks.

These blocks address many bugs found in those existing GNU Radio blocks. These blocks also propagate tags correctly and will resync their tracking on time_est and clock_est tags.

The user can select the timing error detector to use. Decision directed TEDs (like M&M) require the user to specify a slicer constellation (usually digital.constellation_bpsk.base() or digital.constellation_qpsk.base()).

The user can select which interpolating resampler to use. The "PFB with MF" interpolating resampler requires the user to provide taps for matched filtering.

The user can also set, within reason, an arbitrary number of output samples per symbol (1, 2, 3, 4, 5,...).

-Andy

@jmcorgan jmcorgan self-assigned this Apr 23, 2017
@jmcorgan jmcorgan added this to the Release 3.7.12 milestone Apr 23, 2017
@awalls-cx18
Copy link
Contributor Author

The build-bot's giving my last commit, that fixes an example flowgraph, a bum rap:

The following tests FAILED:
	 80 - qa_hier_block2 (Failed)
Errors while running CTest

@marcusmueller
Copy link
Member

marcusmueller commented Apr 23, 2017

Specifically:

 80/210 Test  #80: qa_hier_block2 .......................***Failed    3.32 sec
......................................Segmentation fault (core dumped)

since you didn't touch gr-blocks or runtime at all, my bet would be on some other, unrelated thing, failing. Do we know how to re-run the tests? Pinging @noc0lour on this; I think to issue a rebuild, you have to have buildbot role Admin. Dunno if that is intended per policy?

@awalls-cx18
Copy link
Contributor Author

A hand drawn block diagram of the symbol synchronizer block's design is attached. Hopefully it helps you review the DSP. Let me know if you have questions.

-Andy
Symbol_Synchronizer_block_diagram.pdf

@noc0lour
Copy link
Member

@marcusmueller the qa_hier_block2 is one of the broken test cases. I probably just exclude that one and put it on the "tests to investigate" list.

@jmcorgan
Copy link
Contributor

The qa_hier_block2 is a known troublemaker.

@jmcorgan
Copy link
Contributor

@awalls-cx18 👎 for not using an actual napkin for that diagram.

@awalls-cx18
Copy link
Contributor Author

@jmcorgan: LOL!

I should also point to this PDF about the clock tracking loop (P-I loop filter) transfer function derivation:
https://github.com/awalls-cx18/gr-nwr/blob/master/docs/clock_loop.pdf
to demystify some of the math performed when updating the alpha and beta gains of the loop filter.

@marcusmueller
Copy link
Member

marcusmueller commented Apr 24, 2017 via email

@awalls-cx18
Copy link
Contributor Author

@marcusmueller : Feel free to tikz my hand drawn picture, I didn't have time myself (I barely know tikz and LaTex). That picture is also applicable to the existing GNU Radio M&M clock recovery blocks, MSK timing recovery block, and the PFB clock sync blocks.

Note that that picture shows all the components whether they exist or not in the current configuration. I.e., not all TEDs need a slicer constellation; not all TEDs need derivative input samples; if using the PFB MF interpolating resampler, one doesn't need the external matched filter. The decimators are always there, but in some configurations they are doing nothing (decimating by a factor of 1).

Another subtle point is that the MMSE interpolation filter in GNU Radio is actually implemented as a polyphase filter bank, even though it's not called that explicitly.

-Andy

@awalls-cx18
Copy link
Contributor Author

Come 15 May 2017, I will be too busy to address any comments on this PR until 15 June 2017. Please try to get comments to me by, let's say, 8 May 2017, if you would like me to work them sooner rather than later.

@jmcorgan
Copy link
Contributor

jmcorgan commented May 1, 2017

@awalls-cx18 I have some structural issues with the code organization and placement of header files, but this is all non-DSP and mechanical. I'll be doing deeper review this week, and of course, anyone else is free to review and critique.

Andy Walls added 9 commits July 15, 2017 12:49
The QA test currently outputs failure, which is a QA test
problem, and not a problem with the new class.
The QA tests pass now.

Although setting the tolerance to 0.0103 needs some follow up, to see
if that's acceptable.  We might have a bad set of taps, or it could
just be numerical problems with the QA test functions.
Add interpolating resampler classes for use by the symbol_synchronizer_xx
blocks.  They allow the symbol_synchronizer_xx blocks to provide a user
selectable interpolating resampler implementation.  The base class also
provides interpolating resampler sample phase tracking and management
for the symbol_synchronizer_xx blocks.
@awalls-cx18
Copy link
Contributor Author

awalls-cx18 commented Jul 17, 2017

Made the following changes at the request of @jmcorgan :

  • Rebase on master then force push commit. Done.
  • TED class must be removed from API. Split the enum ted_type to its own small API visible definition. Done.
  • Clock tracking loop class must be removed from API. Done.
  • Interp_differentiator_taps.h must be removed from API. Done.
  • Move interpolating resampler class to gr-digital and it must be removed from API. Split the enum ir_type to its own small API visible definition. Done.
  • The mmse_interpolating_differentiator class could move to gr-digital, since only gr-digital uses it. Or maybe it could be left in gr-filter, and use GR_FILTER_INCLUDE. Done, left in gr-filter.

Remaining to be done of @jmcorgan's requested items:

  • Reformat source code to be consistent with GNURadio coding style: no tabs, 2 space indents, lines end at 70 characters. Anything else? (That's all I could glean from the gnuradio wiki on coding style and Emacs C++ is not a clearly defined specification to work to for a Vim user: I could find no definition of its behavior/parameters.)

I can have astyle fix up most of it, but I really need good specification on the code style. I don't want to play "bring me a rock" with white-space changes. :P

Regards,
Andy

@jmcorgan
Copy link
Contributor

@awalls-cx18 Thanks for taking the time to clean up the code organization. As I said, these were the mechanical changes needed to conform to our API principles. I'll review this now for any showstoppers for merging, and since this is a new block, we can merge it in even if we need to make internal changes to the actual DSP functionality after the fact.

@noc0lour
Copy link
Member

@jmcorgan we could try to agree on a astyle configuration and make that a test (:

@jmcorgan
Copy link
Contributor

This is going in as one squashed commit.

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

Successfully merging this pull request may close these issues.

None yet

4 participants