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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

digital: pfb clock sync: Remove deprecated set_taps() API call #3020

Merged
merged 1 commit into from Jan 6, 2020

Conversation

mbr0wn
Copy link
Member

@mbr0wn mbr0wn commented Jan 6, 2020

This API call was deprecated a long time ago and contained a warning
that it will be removed, so we're now being true to our promise.
This also fixes a compiler error in SWIG if you have old headers
installed that SWIG will find (instead of the ones in the source tree).

Note: I saw this compiler bug and it drove me crazy. SWIG shouldn't be using installed headers 馃挗

This API call was deprecated a long time ago and contained a warning
that it will be removed, so we're now being true to our promise.
This also fixes a compiler error in SWIG *if* you have old headers
installed that SWIG will find (instead of the ones in the source tree).
@rear1019
Copy link
Contributor

rear1019 commented Jan 6, 2020

I encountered this error yesterday as well. The root cause seems to be that SWIG does not support std::uniq_ptr [1][2]. I basically made the same changes as in this pull request to make the code compile, but it still does not compile (neither with your pull request):

/home/*/dev/gnuradio/build/gr-digital/swig/CMakeFiles/digital_swig2.dir/digital_swig2PYTHON_wrap.cxx: In function 'PyObject* _wrap_pfb_clock_sync_ccf_set_taps(PyObject*, PyObject*, PyObject*)':
/home/*/dev/gnuradio/build/gr-digital/swig/CMakeFiles/digital_swig2.dir/digital_swig2PYTHON_wrap.cxx:12669:15: error: 'class gr::digital::pfb_clock_sync_ccf' has no member named 'set_taps'
12669 |       (arg1)->set_taps((std::vector< float,std::allocator< float > > const &)*arg2,*arg3,*arg4);
      |

(repeated three more times at different locations)

It seems like SWIG still tries to wrap set_taps() and I have no idea why. This is probably relevant:

This also fixes a compiler error in SWIG if you have old headers
installed that SWIG will find (instead of the ones in the source tree).

My system has SWIG 4 and a copy of GNU Radio is installed globally (package from distribution鈥檚 repo).

[1] http://swig.org/Doc4.0/SWIGDocumentation.html#CPlusPlus11_general_purpose_smart_pointers
[2] http://swig.org/Doc3.0/SWIGDocumentation.html#CPlusPlus11_general_purpose_smart_pointers

@mbr0wn
Copy link
Member Author

mbr0wn commented Jan 6, 2020

@rear1019 Yeah if you have older headers in your system, SWIG will use those instead of the ones from the source tree. That's a separate issue. Took me quite a bit to figure that out -- I ran two bisects over a lot of commits before it dawned on me.

Copy link
Member

@marcusmueller marcusmueller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine; let me check for usage of this in GRC world, and if none in-tree, merge.

@marcusmueller marcusmueller merged commit 4846be9 into gnuradio:master Jan 6, 2020
@mbr0wn mbr0wn deleted the digital-pfb-remove-set-taps branch August 19, 2020 09:37
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

3 participants