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 initial support for gnuradio 3.10. #682

Merged
merged 3 commits into from
Jun 5, 2022

Conversation

geezer85
Copy link
Contributor

In gnuradio 3.10 the udp_sink "block" moved from gr::blocks to gr::network. This fixes the build failure caused by the move.

@Dygear
Copy link
Contributor

Dygear commented May 15, 2022

Hum. Just to be a little weary of this. The current version of gnuradio on the latest version of Raspberry Pi OS (64bit) Bullseye (Debian 11 based) still downloads gnuradio arm64 3.8.2.0-14.

Copy link
Contributor

@Dygear Dygear left a comment

Choose a reason for hiding this comment

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

This looks OK to me, as the Pi version (3.8) is handled here.

@robotastic
Copy link
Owner

Awesome! I have to find a way to run GR 3.10 to test against. I will go create a Dev Container to test inside of.

@Dygear
Copy link
Contributor

Dygear commented May 30, 2022

Weirdly even Debian (both the arm64 and amd64 builds) still has 3.8 as their default installed version. I just installed Debian on a MiniForum AMD 5800H and it has the exact same version of the Raspberry Pi 4s that I have setup.

@Dygear
Copy link
Contributor

Dygear commented May 31, 2022

I just decommissioned 4 Raspberry Pi 4s. I'm going to install Ubuntu 22.04 LTS (arm64) onto one of them and try to do the build. I'll report back once done.

@Dygear
Copy link
Contributor

Dygear commented Jun 1, 2022

Looks like the iomanip include in openmhz and rdio are not actually needed as well. But it builds cleanly with and without them.

@Dygear
Copy link
Contributor

Dygear commented Jun 1, 2022

Builds cleanly on master, @geezer85 the only recommendation is removing the iomanip from the openmhz and Rdio plugin. It doesn't hurt, but it also doesn't seem to do anything. So if you have a reason for putting it in there if you could explain that would be helpful. Otherwise this all looks good to me and is ready to merged.

Tested on Ubuntu 22.04 arm64 (GR 3.10) and Raspberry Pi OS arm64 (GR 3.8) with and without iomanip in the openmhz and Rdio plugins.

@robotastic

@@ -1,5 +1,8 @@
#include <curl/curl.h>
#include <time.h>
#if GNURADIO_VERSION >= 0x030a00
#include <iomanip>
Copy link
Contributor

Choose a reason for hiding this comment

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

Does not seem to be needed, but also doesn't hurt.

@@ -1,5 +1,8 @@
#include <curl/curl.h>
#include <time.h>
#if GNURADIO_VERSION >= 0x030a00
#include <iomanip>
Copy link
Contributor

Choose a reason for hiding this comment

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

Does not seem to be needed, but also doesn't hurt.

@geezer85
Copy link
Contributor Author

geezer85 commented Jun 1, 2022

I included it because on my setup I got a build error " 'setprecision' is not a member of 'std' " and including iomanip solved it. I only encountered it with GR 3.10. I'm building on Arch Linux ARM using the generic aarch64 image.

Another header must be including it on the Pi, but not in Arch. It's probably more correct to check if the header was already included rather than check the GR version though since it's not really a GR issue.

@robotastic
Copy link
Owner

Oh interesting... what is a good way to solve this? We could just add iomanip.h with a check and it will just be ignored if it is being included through other paths.
https://stackoverflow.com/questions/12453557/setprecision-is-not-a-member-of-std

@geezer85
Copy link
Contributor Author

geezer85 commented Jun 5, 2022

The iomanip header should have include guards to prevent multiple inclusions.

@robotastic
Copy link
Owner

Awesome!! Thanks for making these changes

@robotastic robotastic merged commit 2279fd9 into robotastic:master Jun 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants