drowe67
Port ezDV FreeDV Reporter implementation to freedv-gui
~~This PR embeds the socket.io-client-cpp source tree, including all required submodules, into the freedv-gui codebase (as of commit https://github.com/socketio/socket.io-client-cpp/commit/da779141a7379cc30c870d48295033bc16a23c66). This is being done for several reasons:~~
~~1. The above commit was pushed to the socket.io-client-cpp repo seven months ago as of this writing. This plus the fact that the last official release was in 2021 raises suspicions that it is a dead project.~~ ~~2. Due to a lack of recent release, packaging FreeDV on various distros is far more challenging as many distros are reluctant to package anything newer than the most recent official release. In fact, the version of FreeDV in Debian based distros was patched to disable FreeDV Reporter support entirely precisely due to a lack of any suitable sioclient packages.~~ ~~3. Adding socket.io-client-cpp as a Git submodule adds additional complications during release due to the fact that the tarballs generated by GitHub do not include submodules. Additional manual effort would need to be done every release to provide a suitable tarball for users that wish to compile FreeDV themselves.~~ ~~4. Reimplementing socket.io support (or even using a different protocol for communicating with the FreeDV Reporter server) would be significant additional work that could be spent working on e.g. RADE or other bugfixes.~~
EDIT: I've decided to adapt some of the code used by ezDV for FreeDV Reporter instead. This required some infrastructure code (i.e. TCP socket handling) to be put into place, so the change is still pretty big (but much smaller than if individual sioclient-cpp files were included instead). I made this decision because I noticed issues with sioclient-cpp being unable to reconnect to FreeDV Reporter and didn't really want to have to dig through a possibly dead project to fix the problem.
@barjac, @Tyrbiter, might be good to have your feedback here since you're a lot closer to the whole packaging thing.
EDIT: hmm, 1500 files in this PR? Will have to think about this some more as that's more than I thought there'd be.
Yes, that's a big load of files. It's likely to make maintenance difficult, maybe a cut-down version that doesn't have all the fluff that is in the whole sioclient repo would work. You're probably the person who could work out what is needed @tmiw or it could be hived off to someone with the right skills.
As long as this will not preclude the use of our distro package in the short term, then I have no opinion either way. If you were to create a lighter version as part of freedv code then that would be the best solution, however if it was developed as a lib then we (Mageia) would prefer to package it separately. We generally try to not use embedded 3rd party libs that can be packaged separately in the distro.
As long as this will not preclude the use of our distro package in the short term, then I have no opinion either way. If you were to create a lighter version as part of freedv code then that would be the best solution, however if it was developed as a lib then we (Mageia) would prefer to package it separately. We generally try to not use embedded 3rd party libs that can be packaged separately in the distro.
Based on the current contents of the PR, it wouldn't use the distro package at all, just the version in the source tree. That said, 1500+ files is not ideal so we probably want a different approach regardless.
Anyway, I noticed that most of the files are for asio. Maybe if we can find a way not to have to use that library, it'll cut down the file requirements quite a bit. I'm starting to wonder if just porting the minimal sioclient that ezDV uses (and adding event RX support to support the internal FreeDV Reporter) may be a better approach.
Another option is that I can just have a ZIP file or tarball in the tree instead of the individual files. Then CMake can just use that instead of trying to pull from GH (which is the primary problem in the first place as most package build environments don't have network access). Any reason why that wouldn't work?
Another option is that I can just have a ZIP file or tarball in the tree instead of the individual files. Then CMake can just use that instead of trying to pull from GH (which is the primary problem in the first place as most package build environments don't have network access). Any reason why that wouldn't work?
No, there is no issue with having more than one source tarball in a package. In that case why not just use a fork which can be packaged and added as a dependency as we do in Mageia. Distros who want to package it separately can, and those who want to loosely embed it can include the tarball in the package. You could then control the fork versioning and keep it tied to freedv, which should satisfy the distros who want only full releases for packages. I run a script which is in our libsioclient package sources that creates the tarball with submodules, it's not a problem and only takes about a minute. And yes our build system has no internet connection, packages must be built from supplied sources which are fixed at a version, making builds repeatable and suitable for reliable QA and security.
I went ahead and replaced the individual files with a .zip file containing the sioclient source tree (well, pending successful completion of the build checks anyway). This is because I'm not sure when I'll have time to port over the ezDV implementation so I don't really want this floating around for a while.
If I want to force a build that uses this sioclient tarball, is it fairly simple to do this with a cmake directive?
If I want to force a build that uses this sioclient tarball, is it fairly simple to do this with a cmake directive?
I think you can temporarily rename /usr/include/sio_client.h and that'll force the tarball to be used. There's no specific CMake options otherwise.
Building at #8aab:
Switched to branch 'ms-embed-sioclient' Already up to date. -- Compilation date = XX2025-02-20XX -- freedv-gui current git hash: 8aab
The build is still looking for sioclient and somehow finding it despite it not being installed in the host system.
-- Looking for sioclient... -- Socket.io library: /usr/lib64/libsioclient.so -- Socket.io headers: /usr/include -- Socket.io library found. -- Looking for samplerate...
It's not building it from the tarball. It then fails (correctly) not finding sio_client.h
[ 71%] Building CXX object src/reporting/CMakeFiles/fdv_reporting.dir/FreeDVReporter.cpp.o /home/baz/freedv-rade/freedv-gui/src/reporting/FreeDVReporter.cpp:24:10: fatal error: sio_client.h: No such file or directory 24 | #include "sio_client.h" | ^~~~~~~~~~~~~~ compilation terminated.
All rather confusing as I thought I saw the search for libsioclient removed by this PR.
All rather confusing as I thought I saw the search for libsioclient removed by this PR.
https://github.com/drowe67/freedv-gui/pull/836/commits/ee5dcc2dbeef53bcb355c8366acb7134db1987ff brought back the check for libsioclient. Additionally, based on your previous feedback it sounded like we still needed to have this check to enable use of the distro libsioclient package if one exists?
Anyway, I wonder if the build folder needs to be wiped and build_linux.sh rerun for the changes in this PR to fully take effect. It didn't seem necessary here but I also never installed that library on my system.
I just found that the lib was still installed but the devel was not so all seems OK if the check is still there. Panic over in that case :)
All headers are only in the -devel packages.
Anyway, I wonder if the build folder needs to be wiped and
build_linux.shrerun for the changes in this PR to fully take effect. It didn't seem necessary here but I also never installed that library on my system.
I do this as a matter of course while freedv with RADE cannot easily be packaged in full, but it still builds using the sioclient Fedora packages I have locally.
I suppose that I should really uninstall these sioclient and old freedv packages and then ensure that this build completes and runs correctly, maybe tomorrow or at the weekend :-p
All rather confusing as I thought I saw the search for libsioclient removed by this PR.
ee5dcc2 brought back the check for libsioclient. Additionally, based on your previous feedback it sounded like we still needed to have this check to enable use of the distro libsioclient package if one exists?
Yes that is fine
Anyway, I wonder if the build folder needs to be wiped and
build_linux.shrerun for the changes in this PR to fully take effect. It didn't seem necessary here but I also never installed that library on my system.
I have just made a full rebuild with the system packages installed (installing -devel package obviously pulls in the lib). This built fine at current master.
I then used my update script as I always do for PRs and removed the system sioclient packages before running it.
The script checks out master and then switches to a branch if required and I switched to ms-embed-sioclient branch before running build_linux.sh.
It still appears to find sioclient and does not build it from the tarball, despite the host system not having the packages, but then fails again.
Maybe the only way with this particular branch would be to merge it locally before the full build, but that's not catered for in my script.
EDIT: I hacked my freedv-rade-build script to switch to this branch before building, without the system sioclient packages installed and with them removed from the deps list. The build completed OK with sioclient being built from the tarball. The reporter also performs OK so test complete :)
It would be good to understand why a re-run of build_linux.sh in the venv still 'finds' sioclient when it's not there.
I was looking into the Windows emoji issue earlier and I'm beginning to think there's something wrong with the connection retry logic in sioclient. Maybe it's time to reconsider porting the ezDV implementation over.
This latest commit builds correctly on Fedora 41, running ldd on the binary shows no links to libsioclient.so which means it should be independent of the sioclient packages.
This runs happily, sees the reporter server after failing to connect via IPv6 initially (which I don't have enabled).
I will report further if I see any problems.
The INFO debug output of SocketIoClient: engine.io ping every 5 seconds is reassuring, but probably could be made less noisy :)
Added some stuff to make sure it actually does reconnect when it loses network connection (I turned off Wi-Fi and turned it back on again and that didn't do as I expected before). I also fixed an additional segfault that I noticed. Hopefully that works a bit better for you guys too.
Is this of any consequence, happened when I stopped the modem and closed the gui:
[2025-02-27 23:08:14] [error] handle_read_frame error: websocketpp.transport:7 (End of File) [2025-02-27 23:08:14] [disconnect] Disconnect close local:[1006,End of File] remote:[1006]
Other than that, this build has been running all day without issue.
I have used it today and the reporter has worked fine. I did however have a machine lockup during RX. The RX audio started to stutter for a couple of seconds and the the machine totally locked up. After a power off and restart it continued OK. A second time it happened with FreeDV running but not in use and again it locked up. Hopefully this fixes it but I will watch top in case it's running out of memory or similar. I am using a python audio filter (gnuradio) to clean up the whiskers on the transmitted spectrum. It works really well but maybe adds to the loading. Thanks for the pipewire pointers :)
Is this of any consequence, happened when I stopped the modem and closed the gui:
[2025-02-27 23:08:14] [error] handle_read_frame error: websocketpp.transport:7 (End of File) [2025-02-27 23:08:14] [disconnect] Disconnect close local:[1006,End of File] remote:[1006]
Other than that, this build has been running all day without issue.
I think that's normal because of how websocketpp is being told that the connection's closed.
I have used it today and the reporter has worked fine. I did however have a machine lockup during RX. The RX audio started to stutter for a couple of seconds and the the machine totally locked up. After a power off and restart it continued OK. A second time it happened with FreeDV running but not in use and again it locked up. Hopefully this fixes it but I will watch top in case it's running out of memory or similar.
Yeah, I wouldn't expect a user mode process to fully lock up the system like that. Maybe just to be sure it's not this PR causing it, try the latest v2.0-dev again.
BTW is it me or does it actually connect more quickly to FreeDV Reporter now? In the past it would take a couple of seconds after starting the application but it seems instant now.
It certainly seems to be quicker, although I have not timed it. It appears to use less code and not (so far) to fall over by failing to reconnect when the server is still working. That's 2 positives and no obvious negative.
Another positive is that sioclient-cpp doesn't need to be packaged using a git ID instead of a release tarball. That is a good thing for Fedora at least.
Think this is okay to merge, then? Or should we wait for @barjac to come back with info about whether v2.0-dev works better before doing so?
Think this is okay to merge, then? Or should we wait for @barjac to come back with info about whether v2.0-dev works better before doing so?
It's probably worth seeing what @barjac says, and perhaps given the importance of the reporter it might be worth getting something out to the Windows testers to confirm that this is a good solution for all OSs.
Think this is okay to merge, then? Or should we wait for @barjac to come back with info about whether v2.0-dev works better before doing so?
It's probably worth seeing what @barjac says, and perhaps given the importance of the reporter it might be worth getting something out to the Windows testers to confirm that this is a good solution for all OSs.
OK, I'll wait for his response. In the meantime, I forwarded the most recent build GitHub generated over to Walter for further Windows testing.
Haven't heard anything from the other test group yet so this is likely fine. @barjac, how is this working for you?
Running https://github.com/drowe67/freedv-gui/pull/836/commits/51fdca36a75d65cd21b163844451ae7393090d01 here now, and it seems to be behaving well without any noticeable problems.
Tiny thing, when the main window shows <mode>-D next to the Clear button, could a couple of pixels be squeezed in to separate the two elements a little?
Running 51fdca3 here now, and it seems to be behaving well without any noticeable problems.
Tiny thing, when the main window shows
-D next to the Clear button, could a couple of pixels be squeezed in to separate the two elements a little?
Done.
Yes this seems to be working well however there are quite a few Warnings and Notes during the build mainly related to superfluous '<...>' IIRC.
Oh and sorry for the delay in replying to this - been busy with other things.
/OT I have been asked in an 'issue' in freedv-rade-build whether build_linux.sh could use parallel builds where the hardware has the capability. TBH I'm not over keen on this as some projects simply won't build using multi-cores, however if you think it is reasonable in this case then would it be worth an 'issue'? I did try a mod to my script to change the 'make' commands visible in build_linux.sh and immediately ran into problems, so, for the time it 'may' save during a build I have my doubts that it's worth the effort.
Yes this seems to be working well however there are quite a few Warnings and Notes during the build mainly related to superfluous '<...>' IIRC.
Thanks, I think the warnings are inside websocketpp. Until they get upgraded to errors, I don't necessarily want to be making local changes to those. I'll go ahead and merge.
Oh and sorry for the delay in replying to this - been busy with other things.
No worries!
/OT I have been asked in an 'issue' in freedv-rade-build whether build_linux.sh could use parallel builds where the hardware has the capability. TBH I'm not over keen on this as some projects simply won't build using multi-cores, however if you think it is reasonable in this case then would it be worth an 'issue'? I did try a mod to my script to change the 'make' commands visible in build_linux.sh and immediately ran into problems, so, for the time it 'may' save during a build I have my doubts that it's worth the effort.
I think you should be able to just append -j to the last line of build_linux.sh, i.e.
make VERBOSE=1 -j
This should automatically determine the optimal number of parallel builds. You can also force a certain number of cores by appending a number after -j, for example -j6 for 6 cores.