search

gnuradio / gnuradio

GNU Radio – the Free and Open Software Radio Ecosystem
https://gnuradio.org
GNU General Public License v3.0
5.12k stars 1.91k forks source link

gnuradio next running a fm demodulator fails due to broken division #1902

Closed dl1ksv closed 6 years ago

dl1ksv commented 6 years ago

A fm demodulator fails with

File "/usr/local/gnuradio/lib/python3.6/dist-packages/gnuradio/analog/fm_demod.py", line 67, in init 60 # Stopband attenuation File "/usr/local/gnuradio/lib/python3.6/dist-packages/gnuradio/filter/optfir.py", line 54, in low_pass passband_dev = passband_ripple_to_dev (passband_ripple_db) File "/usr/local/gnuradio/lib/python3.6/dist-packages/gnuradio/filter/optfir.py", line 188, in passband_ripple_to_dev return (10(old_div(ripple_db / 20)-1),(10(ripple_db / 20)+1)) NameError: name 'old_div' is not defined

Which kind of division should be used ? / or //

Is / correct ?

kpreid commented 6 years ago

I hit this bug too. The function old_div is not defined anywhere in the source tree, but called in several files. The calls were introduced as part of Python 3 conversion in 9e625c4821f4c63421b3d3747c0c4f358fef6c5f. Presumably the intent was to replicate the behavior of the old Python 2 / operator which truncates if and only if the inputs are integers.

A web search indicates that there is a module past.utils which provides this function, part of the Python-Future package, but this module is not imported at all.

The affected files are:

gnuradio-runtime/lib/math/gen_sine_table.py
gr-trellis/docs/test_tcm.py
gr-trellis/examples/python/test_tcm.py
gr-fec/python/fec/fec_test.py
gr-filter/python/filter/optfir.py
gr-digital/python/digital/ofdm_receiver.py
gr-digital/python/digital/gmsk.py
gr-digital/examples/example_timing.py

Looking over the instances, nearly all of them could be replaced trivially with / because the arguments are guaranteed to be float, and the rest are broken (passing only one argument, usually because of misplacing the call or the closing parenthesis compared to the previous code).

noc0lour commented 6 years ago

Sounds about right. Do you want to put up a PR fixing this issue?

kpreid commented 6 years ago

Preparing a patch may or may not fit into my free time; I can't say right now.

Why wasn't this caught by tests?

noc0lour commented 6 years ago

Looks like none of the affected files is run in tests and thus are not verified.

noc0lour commented 6 years ago

Also going through the code none of the old_div calls is syntactically correct ... so it wouldn't have ever run, even if old_div is imported.

noc0lour commented 6 years ago

See #1942

marcusmueller commented 6 years ago

@dl1ksv @kpreid I've merged #1942 . My hope is that this issue is thus fixed. Can you confirm?

dl1ksv commented 6 years ago

@marcusmueller Yes, this issue is fixed !

noc0lour commented 6 years ago

@dl1ksv thanks for testing!