Re: Python Bindings Out of Sync: Which files are out of sync?

[Glen Langston]

Dear Josh and all Gnuradio-ers,

Thanks your hint helped me fix the problem.    

The previous code was written using swig, so I’d not run the
gr_modtool bind <oot module> 
program like that before.

All is OK and I have two, very simple, new modules for my purposes.
One (vselect) just selects a sub-channel range.  (Probably this already
exists, but I couldn’t find it).   That vselect takes a vector of some
length and outputs a shorter vector, synchronously.   The output vector
is selected based on two input arguments.

For others, the one confusing thing (for me) about gr_modtool is the directory
to run it in.

The files I needed to act on are in

~/test/include/gnuradio/radio_astro/vselect.h

and 

~/test/lib/vselect_impl.cc <http://vselect_impl.cc/> and vselect_impl.h

for the module “vselect”

To fix the problem I had, required executing the command the
top module directory.  Ie :

cd ~/test

gr_modtool bind vselect

The input files are found and the new file is created in directory

~/test/python/radio_astro/bindings

new file is vselect_python.cc <http://vselect_python.cc/>

One must also edit python_bindings.cc <http://python_bindings.cc/> to add this 
new module to the list. (At least I had to).

Best regards,

Glen

In the Swig era, One had to edit files by hand.


> On Jan 5, 2024, at 9:48 AM, Josh Morman <jmorman@gnuradio.org> wrote:
> 
> When cmake is run, it does an md5 hash on the public header files and makes 
> sure the value matches what is written in *_python.cc
> Even a single character change in the .h file will disturb the hash and 
> require a rebinding.  If you are sure that you don't want to rebind, then you 
> can call:
> gr_modtool bind -u blockname
> This will only update the hash and not regenerate the *_python.cc file
> 
> Ignoring these errors via a cmake flag would be possible, but it is not 
> implemented currently.
> 
> 
> On Fri, Jan 5, 2024 at 8:14 AM Glen Langston <glen.i.langston@gmail.com> 
> wrote:
> Thanks Josh for your quick response.
> 
> I’ve written c++ code for gnuradio successfully before.   I’m not
> sure what is different about this, very simple, code.
> 
> What does “out of sync” mean specifically?   I’ve got the same calling
> arguments in both files, modulo the different programming styles for
> python and c++.  
> 
> Is there an “ignore this error” flag in “cmake”?
> 
> Thanks
> 
> Glen
> > On Jan 4, 2024, at 4:52 PM, Josh Morman <jmorman@gnuradio.org> wrote:
> > 
> > Hi Glen,
> > 
> > The file that is out of sync is 
> > vsum_python.cc
> > 
> > This can be updated using 
> > gr_modtool bind vsum
> > 
> > This ensures that changes in vsum.h are reflected in the python bindings.
> > 
> > Josh
> > 
> > On Thu, Jan 4, 2024 at 2:05 PM Glen Langston <glen.i.langston@gmail.com> 
> > wrote:
> > Hello
> > 
> > I’m running gnuradio 3.9.x and am writing a couple simple C++ blocks.
> > 
> > in ~/test/lib 
> > 
> > I have vsum_impl.cc <http://vsum_impl.cc/> and vsum_impl.h
> > 
> > In ~/test/python/radio_astro/bindings
> > 
> > I have vsum.h
> > 
> > I get these messages in the build “make ..” step:
> > 
> > ...
> > -- PYTHON and GRC components are enabled
> > -- Python checking for pygccxml - not found
> > -- Found PythonLibs: /usr/lib/aarch64-linux-gnu/libpython3.9.so
> > -- Performing Test HAS_FLTO
> > -- Performing Test HAS_FLTO - Success
> > -- Found pybind11: /usr/include (found version "2.6.2" )
> > CMake Error at /usr/local/lib/cmake/gnuradio/GrPybind.cmake:221 (message):
> >   Python bindings for vsum.h are out of sync
> > Call Stack (most recent call first):
> >   python/radio_astro/bindings/CMakeLists.txt:39 (GR_PYBIND_MAKE_OOT)
> > 
> > 
> > -- Configuring incomplete, errors occurred!
> > See also 
> > "/home/radioastro/Research/gr-radio_astro/build/CMakeFiles/CMakeOutput.log".
> > See also 
> > "/home/radioastro/Research/gr-radio_astro/build/CMakeFiles/CMakeError.log".
> > 
> > Could someone remind me exactly which files are “out of sync”?
> > 
> > vsum.h must be one, but which is the other?   I think that vsum_impl.h and 
> > vsum.h are consistent
> > (but not, of course, identical).
> > 
> > Thanks!
> > 
> > Glen
> > 
> > 
> > 
> > > On Dec 30, 2023, at 8:16 AM, Marcus Müller <marcus.mueller@ettus.com> 
> > > wrote:
> > > 
> > > Heyo Kimmo,
> > > sorry for the delayed response:
> > > On 29.12.23 01:00, Kimmo Lehtinen wrote:
> > >> I would like to make modifications to the following two GNURadio blocks:
> > >> 1) QT GUI number sink-------------------------------
> > >> I would like to modify it so that it can also display integers and 
> > >> strings. Currently it can display floats, shorts and bytes.
> > >> I raised an issue about this at the Github page of GNURadio, and I got 
> > >> the following reply:"A number of GR blocks infer type from item size, 
> > >> and that's what this block does (in its constructor). Unfortunately, 
> > >> float and int32 are the same size, so int32 is not usable.It would be 
> > >> possible to add another constructor that uses an explicit type instead 
> > >> of item size."
> > > Warning: this is probably more involved than you hoped for. If you've 
> > > worked with C++ before: No problem, you can at any point always ask for 
> > > help. It's also super helpful to use "Draft PR" on github to share your 
> > > current state of affairs!
> > > If you haven't: I think this might be a bit too hard.
> > > Yep, you would need to copy the make function as declared in 
> > > number_sink.h in [0]:
> > >     static sptr make(size_t itemsize,
> > >                      float average = 0,
> > >                      graph_t graph_type = NUM_GRAPH_HORIZ,
> > >                      int nconnections = 1,
> > >                      QWidget* parent = NULL);
> > > 
> > > to a second make function that has a different signature, for example
> > >     static sptr make(item_type_t itemtype,
> > >                      float average = 0,
> > >                      graph_t graph_type = NUM_GRAPH_HORIZ,
> > >                      int nconnections = 1,
> > >                      QWidget* parent = NULL);
> > > where item_type_t is a "Scoped enum"/class enum [1]; something like, 
> > > within number_sink class,
> > > enum class item_type_t { FLOAT, INT32 , INT16, INT8 }; // or whatever 
> > > types you want to support
> > > Then you would actually need to implement that in number_sink_impl.cc 
> > > like [2]. But for that you need to modify the actual constructor to not 
> > > take size_t itemsize as argument [3], but item_type_t itemtype!
> > > You would add a field const item_type_t d_itemtype and remove d_itemsize 
> > > in number_sink_impl.h [4] and add it to the initializer list [5]; you'd 
> > > want a switch()/case construct to set the alignment_multiple [6].
> > > Then, you replace the switch (d_itemsize) in get_item [7] with an 
> > > appropriate switch(d_itemtype). 
> > > Test it successfully compiles!
> > > Now you only need to do two things to 
> > > gr-qtgui/python/qtgui/bindings/number_sink_python.cc 
> > >     • add the new class enum item_type_t to bind_number_sink [8],
> > >     • add the new make function:
> > >         • modify the existing definition and
> > >         • copy it to replace size_t itemsize with 
> > > number_sink::item_type_t itemtype
> > > In detail: following [9], you need to change
> > >     py::class_<number_sink,
> > >                gr::sync_block,
> > >                gr::block,
> > >                gr::basic_block,
> > >                std::shared_ptr<number_sink>>(m, "number_sink", 
> > > D(number_sink))
> > > 
> > >         .def(py::init(&number_sink::make), 
> > >         …………………
> > > 
> > > into
> > >     py::class_<number_sink,
> > >                gr::sync_block,
> > >                gr::block,
> > >                gr::basic_block,
> > >                std::shared_ptr<number_sink>>
> > >         number_sink_wrapper(m, "number_sink", D(number_sink));
> > > 
> > >     py::enum_<number_sink::item_type_t>(number_sink_wrapper, 
> > > "item_type_t");
> > > 
> > >     number_sink_wrapper
> > >         .def(py::init(&number_sink::make),
> > >         ……………………
> > > Please compile the result (make sure to make clean before, we've changed 
> > > bindings), and test it works, from python!
> > > Now, we need to tell GRC that there's a new way to create a Qt GUI number 
> > > sink! So, modify qtgui_humber_sink.block.yml [10] to use the new make 
> > > function instead of the old (both in the python templates:       
> > > directive, and in the C++ cpp_templates: directive), and add the integer 
> > > option to id: type. Then make; make install and open GRC and try!
> > > Definitely make sure you also add an example GRC flowgraph to the 
> > > gr-qtgui/examples director.
> > > Then, please make a commit with a title that starts with "qtgui: ", for 
> > > example "qtgui: add type-based number_sink, allowing for int32", and 
> > > don't forget to use the -s flag with git commit, so that your commit is 
> > > signed off! Push to your fork of GNU Radio, and open a Pull Request 
> > > against main.
> > > [0] 
> > > https://github.com/gnuradio/gnuradio/blob/86af478a88cf0521b023ad2c924c74ea38d9adcf/gr-qtgui/include/gnuradio/qtgui/number_sink.h#L55-L68
> > > [1] https://en.cppreference.com/w/cpp/language/enum#Scoped_enumerations
> > > [2] 
> > > https://github.com/gnuradio/gnuradio/blob/86af478a88cf0521b023ad2c924c74ea38d9adcf/gr-qtgui/lib/number_sink_impl.cc#L38-L43
> > > [3] 
> > > https://github.com/gnuradio/gnuradio/blob/86af478a88cf0521b023ad2c924c74ea38d9adcf/gr-qtgui/lib/number_sink_impl.cc#L46
> > > [4] 
> > > https://github.com/gnuradio/gnuradio/blob/86af478a88cf0521b023ad2c924c74ea38d9adcf/gr-qtgui/lib/number_sink_impl.h#L28
> > > [5] 
> > > https://github.com/gnuradio/gnuradio/blob/86af478a88cf0521b023ad2c924c74ea38d9adcf/gr-qtgui/lib/number_sink_impl.cc#L50
> > > [6] 
> > > https://github.com/gnuradio/gnuradio/blob/86af478a88cf0521b023ad2c924c74ea38d9adcf/gr-qtgui/lib/number_sink_impl.cc#L64
> > > [8] 
> > > https://github.com/gnuradio/gnuradio/blob/86af478a88cf0521b023ad2c924c74ea38d9adcf/gr-qtgui/python/qtgui/bindings/number_sink_python.cc
> > > [9] 
> > > https://pybind11.readthedocs.io/en/stable/classes.html#enumerations-and-internal-types
> > > [10] 
> > > https://github.com/gnuradio/gnuradio/blob/86af478a88cf0521b023ad2c924c74ea38d9adcf/gr-qtgui/grc/qtgui_number_sink.block.yml
> > >> If displaying strings, there is no sense to have a bar graph, which is 
> > >> currently an option in the 'QT GUI number sink' block. Thus, there could 
> > >> be a separate block for displaying strings, without an option for a bar 
> > >> graph. 
> > > Qt GUI Label can already do that; since there's no possibility to send 
> > > strings on GNU Radio streams, the question is more: where do your strings 
> > > come from!
> > >> 2) QT GUI time raster sink-----------------------------------
> > >> I would like to modify it so that the parameters 'x-axis start value' 
> > >> and 'x-axis end value' can be changed during runtime. 
> > > sounds like a good idea.
> > >> For example, in the 'QT GUI vector sink' block the corresponding 
> > >> parameters can be changed during runtime. 
> > >> 
> > >> I understand that the best way to make the modifications is to change 
> > >> the code and then recompile the whole GNURadio.
> > > yes
> > >> 
> > >> The problem is that I cannot write C++ code...
> > > So, you really can't :( I think this really means that hopefully someone 
> > > with time finds this email and implements that!
> > >> Is there anyone willing to change the code ?   Or at least give good 
> > >> instructions how to do that. 
> > >> 
> > >> I think these modifications would be useful also to other GNURadio users 
> > >> :-)
> > > Ideally, you'd open a Feature Request on github,
> > > https://github.com/gnuradio/gnuradio/issues/new?assignees=&labels=Feature+Request&projects=&template=feature_request.yml
> > > You can simply copy and paste your email into feature description, and 
> > > paste my reply into "More information", preceded by "Marcus says:" 
> > > 
> > > Best,
> > > Marcus
> > 
> > 
>