Sub-classing a block defined in an _impl class

GNU Radio
1

Hello,

this question my arise from my ignorance about C++, but I don’t
understand how, if it is possible, to sub-class a GNURadio block that
has moat of his code in an _impl class.

To my understanding I would need to subclass both the class defining the
block and his _impl class. However, it seems that the _impl class is not
part of the exposed API, therefore there is no way to subclass it. There
is way to achieve this without copying around significant portions of
code?

The case at hand is subclassing gr::analog::pll_carriertracking_cc
changing the phase_detector() method to use the regular atan2() instead
of the fast version.

Thanks!

Cheers,
Daniele

2

You’re right – that’s one of the reasons why we try and keep the
block’s guts (their kernels) elsewhere than the actual block definition.

Not all of our blocks do this, though.

M

3

Hello Martin,

I’m not sure I understand. Are you saying that there is no way of doing
what I want without copying over to my class the implementation of the
block that is in the _impl class?

Why is this a good thing?

Cheers,
Daniele

4

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Daniele,

_impl classes are not meant to be subclassed usually.
GR distinguishes between interface or public headers, which are
located in the include directory and private headers which are located
in the lib directory.
You can just add new classes to your lib folder and use them within
this folder. But if you want to export these classes and make them
accessable from Python, you need a public header for it.

On 10.10.2014 12:03, Daniele N. wrote:

around significant portions of code?
The public description of a block is in the corresponding include
folder. It is pure virtual to make sure it remains as an interface file.

The case at hand is subclassing gr::analog::pll_carriertracking_cc
changing the phase_detector() method to use the regular atan2()
instead of the fast version.
You may just change this in GR directly. You have a personal copy of
the code so there is nothing that should contradict this approach.
Unless you want to compare the two but even then you should be able to
accomplish that.

I hope that gives you an idea of the structure and may help you solve
your problem.

Cheers
Johannes
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBAgAGBQJUN7P9AAoJEO7fmkDsqywMpggP/0dQEXmkffTmzeg4cvW5xlmJ
RWzT+eEA1prFMMR3iBSTrlPnLMyQfVz4nxE63AbBx+pEBsHzu6fUUHWZLtlzVwpy
Bq89/Sd5G8sM40TWGKrFfUjxd5ND/4HuKar6JC/Wx9tnfZYCa3q3f3O2vi1S/6wu
Ir6xKGpSGsR9Ro3RZ8iL+d/WoxAv86cH+GxFdo6R496hkqi1gZYdVkBJO1vCAL6S
8s2mvNDkSTcXaGcrF3VhhIBnjcwvhn3GYQMCjeSbcbyk++dLhZH+dmitB27lv6qW
/u/eTNtXVGN2TzzQ0hLW4YI8pLGOj/cENHbBc31zUzRbRwt/ebOWC78bx9dyKALr
WB0kDVM2XI9veYeyxMwvdtFncO7B7MGh+ocozhg53WaWijnFLGwD55yxZedXNcEr
iIdq9Px7smsJ87s6ABgB7MfrhGgxT0ZJxop5D68HBLE4/v1OnRxjfi1V0JpKmIxt
uueDgbKWGQEaDrgQX0BnrQ6bA68Z1HuTHk68j6WOnZ36a2mAZZfnZ8iKVckX/PJv
CFhJLFtTQpfg3DR7lEN7x1gwbyyQGEk6w68sVK+e/LdPc5BHO9d4AcVCgfLaO/Ov
i3QYb+Oqjv7P+jl2N2nAohh+AyNjwSRm2DxQGGwnvbhfChii5Cr5mIc+3y/JeoVf
Vm7RMlaj1ESE9XAifvQC
=z5WH
-----END PGP SIGNATURE-----

5

Hi,

There
is way to achieve this without copying around significant portions of code?

No, you’ll need to copy the _impl

The case at hand is subclassing gr::analog::pll_carriertracking_cc
changing the phase_detector() method to use the regular atan2() instead
of the fast version.

Actually this is a private non-virtual method, so even if all that was
in the public API you wouldn’t be able to do it.

Cheers,

Sylvain
6

On 10/10/14 12:25, Johannes D. wrote:

_impl classes are not meant to be subclassed usually.
GR distinguishes between interface or public headers, which are
located in the include directory and private headers which are located
in the lib directory.
You can just add new classes to your lib folder and use them within
this folder. But if you want to export these classes and make them
accessable from Python, you need a public header for it.

That was my understanding. What I still don’t understand is why things
are done in this way.

On 10.10.2014 12:03, Daniele N. wrote:

The case at hand is subclassing gr::analog::pll_carriertracking_cc
changing the phase_detector() method to use the regular atan2()
instead of the fast version.

You may just change this in GR directly. You have a personal copy of
the code so there is nothing that should contradict this approach.
Unless you want to compare the two but even then you should be able to
accomplish that.

Changing GNURadio code directly makes is very hard to redistribute my
code, and I would like to retain the possibility to use the original
GNURadio block.

I don’t have a problem copying over the code and changing the methods I
want to change, it just does not feel as the most elegant solution.

Cheers,
Daniele

7

The good part is the separation of the API from the implementation. The
not so good thing is that the implementations themselves often contain
all the logic, usually because they are fairly simple. So, your choices
are to subclass or copy/edit the implementation files. [Just saw
Sylvain’s not about phase_detector() being privite - OK].

Also, the implementations are free to change without affecting the API.
Guaranteeing stability of implementations (even the _impl.h) makes the
implementations part of the API, which would be bad. If there were a
family of carrier tracking algorithms, it would make sense to have them
subclass from a common abstract class.

In more complicated blocks, like some in the filter area, a good portion
of the logic is separated out and reusable.

The

8

On 10/10/14 12:30, Sylvain M. wrote:

Hi,

There
is way to achieve this without copying around significant portions of code?

No, you’ll need to copy the _impl

Ok. Thanks for confirming my understanding.

I copied the _impl definition, however, I would prefer to do not copy
the interface definition, therefore I defined my block as follows:

namespace gr {
namespace baz {

class BAZ_API pll_carriertracking_cc
  : public gr::analog::pll_carriertracking_cc
{
public:
  typedef boost::shared_ptr<pll_carriertracking_cc> sptr;
  static sptr make(float loop_bw, float max_freq, float min_freq);
};

} // namespace baz
} // namespace gr

Then I copied the definition of the pll_carriertracking_cc_impl class
implementation into my project and changed it where I wanted.

However, in this way, swig is not happy:

src/gr-baz/swig/baz_swigPYTHON_wrap.cxx: In function ‘PyObject*
_wrap_new_pll_carriertracking_cc(PyObject*, PyObject*)’:
/home/manip/src/gr-baz/swig/baz_swigPYTHON_wrap.cxx:11071:87: error:
cannot allocate an object of abstract type
‘gr::baz::pll_carriertracking_cc’
result = (gr::baz::pll_carriertracking_cc *)new
gr::baz::pll_carriertracking_cc();

But if I copy the definition of my block from the GNURadio definition,
swig is happy, the compilation goes fine, and I can correctly use the
block from Python.

What am I doing wrong? Sorry if this is a trivial C++ error, but my C++
is definitely rusty…

Thanks!

Cheers,
Daniele

9

On 10/10/14 14:29, Daniele N. wrote:

  typedef boost::shared_ptr<pll_carriertracking_cc> sptr;
  static sptr make(float loop_bw, float max_freq, float min_freq);
};

} // namespace baz
} // namespace gr

Then I copied the definition of the pll_carriertracking_cc_impl class
implementation into my project and changed it where I wanted.

However, in this way, swig is not happy:

It seems to be a limitation of SWIG (which I don’t know much, so I may
still be doing something wrong). If I define a virtual method in the
definition of my block interface, SWIG generates the correct C++ code.

I ended up adding a definition for the work() virtual method to fix the
issue. Again, not the most elegant thing to do, but it works…

Cheers,
Daniele

10

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I think it might be more of a C+±inherent thing:
If you declare a method in a class:

class daniele_s_class {
public:
void my_method(int argument);
};

Then you’ll have to either implement that method, or can’t instantiate
the class – which is kind of correct, if you think about it, because
that class is simply incomplete. But it’s not too bad, either, because
noone ever should instantiate daniele_s_class, it’s just meant to
define the interface. That’s why we have the “make”, which will return
a new instance of the _impl subclass. So, schematically, the GNU Radio
way of doing this is:

class daniele_s_class {
public:
virtual void my_method(int argument) = 0; //this tells the compiler
that there is a method that a instantiable subclass must implement
static sptr make();
};

daniele_s_class::sptr daniele_s_class::make(){
return sptr(new daniele_s_class_impl());
}

class daniele_s_class_impl{
public:
virtual void my_method(int argument);
};

void daniel_s_class_impl::my_method(int argument){
//do whatever here :slight_smile:
}

Greetings,
Marcus

On 10.10.2014 15:24, Daniele N. wrote:

make(float loop_bw, float max_freq, float min_freq); };
may still be doing something wrong). If I define a virtual method
_______________________________________________ Discuss-gnuradio
mailing list [email protected]
Discuss-gnuradio Info Page

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJUN/MMAAoJEAFxB7BbsDrLE50H/3QvMRf996L3SA9UGAJ6mKIz
1Dd3mfrLN05kgm75UVr1HYwlbEMpZykL7g5sYyIgB7wOfiLKFWElUHQ03HwPFjFK
MAT7SS/l4Pd8DrSwJefqTARHWCaqSM2Y/rv8eu3eSh9/IvaZNZqDfeWMF5xxtuuD
2gbilF77Dh5jSiYMGjXuMpoGgYcdsLznAvSmLft2a6/k0mX1EcsbNu4VWD8++wzO
rcyv09Da+szs/SY/GwweLl+qFTQCLGMmQg36Ol4SmWpfeXpkMTE1IHaKh2OgzK63
ghNcRPSjZE2l4NKMk+bOSPq8gaLT+IQW2crLHxszVNEhScCPjlRx3IJ342DM+yY=
=51/2
-----END PGP SIGNATURE-----