Fix liblo 0.32 header compatibility #1954

cbix · 2024-03-30T12:05:21Z

Fixes build error: OSCNode.cpp:153:28: error: cannot convert 'lo_arg*' to 'lo_blob' {aka 'lo_blob_*'} in initialization #1949
Doesn't break building with liblo 0.31
0.32 also introduces address pattern matching, which could make the OSCNode code even simpler (?)

cbix · 2024-03-30T12:07:21Z

plugins/osc/OSCNode.cpp

@@ -150,7 +150,7 @@ int OSCDataHandler(const char *osc_address, const char *types, lo_arg **argv,

  if (argc == 1) {
    if (type == "b") {
-      lo_blob blob = argv[0];
+      lo_blob blob = (lo_blob)argv[0];


Example code in liblo also uses a regular typecast:

https://github.com/radarsat1/liblo/blob/c1a51bca21e8535ce77a9daf256f2e74c1a7e80f/examples/example_server.c#L118:L131

kripton

Hi, and thanks for the fix. I was looking for some VERSION symbol in liblo's headers so we could check which version we are building against. However, only a function is provided.
I can confirm that the fix works fine with liblo-0.31 and liblo-0.32, thus I approve the changes.

@peternewman and @nomis52 : Are any older versions of liblo known that we might need to support and that would not compile with these changes?

cbix · 2024-03-30T21:03:27Z

CI debian sid failure seems unrelated:

autopkgtest [20:52:13]: test command3: set -e ; for py in $(py3versions -s 2>/dev/null) ; do echo "Testing with $py:" ; $py $(which rdm_responder_test.py) --help ; done
autopkgtest [20:52:13]: test command3: [-----------------------
Testing with python3.12:
bash: line 1: python3.12: command not found
autopkgtest [20:52:14]: test command3: -----------------------]
autopkgtest [20:52:14]: test command3:  - - - - - - - - - - results - - - - - - - - - -
command3             FAIL non-zero exit status 127

kripton · 2024-03-30T21:16:58Z

CI debian sid failure seems unrelated:

autopkgtest [20:52:13]: test command3: set -e ; for py in $(py3versions -s 2>/dev/null) ; do echo "Testing with $py:" ; $py $(which rdm_responder_test.py) --help ; done
autopkgtest [20:52:13]: test command3: [-----------------------
Testing with python3.12:
bash: line 1: python3.12: command not found
autopkgtest [20:52:14]: test command3: -----------------------]
autopkgtest [20:52:14]: test command3:  - - - - - - - - - - results - - - - - - - - - -
command3             FAIL non-zero exit status 127

Yes, indeed, that python3.12-related failure is not new, so nothing you broke :)

peternewman · 2024-03-31T00:52:06Z

Hi, and thanks for the fix. I was looking for some VERSION symbol in liblo's headers so we could check which version we are building against. However, only a function is provided.

Yeah sadly nothing easy like that and they haven't bumped in such a way we could detect a breaking ABI change either.

I can confirm that the fix works fine with liblo-0.31 and liblo-0.32, thus I approve the changes.
@peternewman and @nomis52 : Are any older versions of liblo known that we might need to support and that would not compile with these changes?

I've got an old box with liblo-0.28 still kicking about and it built fine on there too, so I think we're probably fine from that perspective.

peternewman

Thanks @cbix LGTM/still compiles for me on an older version.

It would be good to get this into 0.10 branch too, but one of us can cherry-pick/backport it if you don't want to rebase onto there.

Just to have a note of it, have you (or indeed @kripton ) tested the OSC functionality with this patch or just that it compiles?

cbix · 2024-03-31T11:16:23Z

have you tested the OSC functionality

not 100 % sure how to test it, I just started olad and ran a python-osc example against its port 7770, couldn't observe any issues or errors but also nothing happens inside ola_dmxmonitor

peternewman · 2024-04-01T22:02:06Z

not 100 % sure how to test it, I just started olad and ran a python-osc example against its port 7770, couldn't observe any issues or errors but also nothing happens inside ola_dmxmonitor

So it depends which direction you're testing @cbix . There's more detail in the plugin's docs (available from olad or online here):
https://docs.openlighting.org/ola/conf/ola-osc.conf.html

So by default it should listen on:
https://github.com/OpenLightingProject/ola/blob/aee74a22b88c12b47a52949e828e670f521385ca/plugins/osc/OSCPlugin.cpp#L47C53-L47C69

From memory the input side accepts any of the formats of the output side.

I think I've used oscdump from liblo-tools to look at data being sent from OLAd in the past.

Remember to patch the OSC port to an OLA universe before checking dmxconsole/dmxmonitor (but I suspect you're aware of that bit already).

cbix · 2024-04-02T00:03:52Z

Got it, OSC sending and receiving works! Tested with oscdump for output ports and python-osc with random floats and 512 byte blobs for input ports:

Screencast.from.2024-04-02.02-01-56.webm

peternewman · 2024-04-02T08:06:50Z

Perfect thanks @cbix !

Do you want to try and rebase this onto the 0.10 branch, otherwise we can merge what you've got and backport it ourselves?

cbix · 2024-04-02T08:28:43Z

@peternewman done :)

peternewman

LGTM thanks @cbix 💯 !

kripton · 2024-04-02T11:22:51Z

Would it be possible to release an 0.10.10 with that fix?

Closes: https://bugs.gentoo.org/927000 Upstream-PR: OpenLightingProject/ola#1954 Upstream-Commit: OpenLightingProject/ola@e083653 Signed-off-by: Jannis Achstetter <[email protected]>

Changes to the PR made by marecki at merge time: - run ola-0.10.9-liblo.patch through scrub-patch - revert the revision bump as not needed while fixing build failures Closes: https://bugs.gentoo.org/927000 Upstream-PR: OpenLightingProject/ola#1954 Upstream-Commit: OpenLightingProject/ola@e083653 Signed-off-by: Jannis Achstetter <[email protected]> Signed-off-by: Marek Szuba <[email protected]>

cbix commented Mar 30, 2024

View reviewed changes

cbix mentioned this pull request Mar 30, 2024

ola: liblo compatibility osam-cologne/archlinux-proaudio#396

Merged

kripton approved these changes Mar 30, 2024

View reviewed changes

peternewman approved these changes Mar 31, 2024

View reviewed changes

Fix liblo 0.32 header compatibility

e083653

cbix force-pushed the fix/liblo-032 branch from a7643c5 to e083653 Compare April 2, 2024 08:24

cbix changed the base branch from master to 0.10 April 2, 2024 08:25

peternewman added this to the 0.10.10 milestone Apr 2, 2024

peternewman approved these changes Apr 2, 2024

View reviewed changes

peternewman enabled auto-merge April 2, 2024 10:57

peternewman merged commit 578b2fb into OpenLightingProject:0.10 Apr 2, 2024
19 of 20 checks passed

chenrui333 mentioned this pull request Jul 22, 2024

ola: declare indirect deps with linkage Homebrew/homebrew-core#178091

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix liblo 0.32 header compatibility #1954

Fix liblo 0.32 header compatibility #1954

cbix commented Mar 30, 2024 •

edited

Loading

cbix Mar 30, 2024 •

edited

Loading

kripton left a comment •

edited

Loading

cbix commented Mar 30, 2024

kripton commented Mar 30, 2024

peternewman commented Mar 31, 2024

peternewman left a comment

cbix commented Mar 31, 2024

peternewman commented Apr 1, 2024

cbix commented Apr 2, 2024

peternewman commented Apr 2, 2024

cbix commented Apr 2, 2024

peternewman left a comment

kripton commented Apr 2, 2024

Fix liblo 0.32 header compatibility #1954

Fix liblo 0.32 header compatibility #1954

Conversation

cbix commented Mar 30, 2024 • edited Loading

cbix Mar 30, 2024 • edited Loading

Choose a reason for hiding this comment

kripton left a comment • edited Loading

Choose a reason for hiding this comment

cbix commented Mar 30, 2024

kripton commented Mar 30, 2024

peternewman commented Mar 31, 2024

peternewman left a comment

Choose a reason for hiding this comment

cbix commented Mar 31, 2024

peternewman commented Apr 1, 2024

cbix commented Apr 2, 2024

peternewman commented Apr 2, 2024

cbix commented Apr 2, 2024

peternewman left a comment

Choose a reason for hiding this comment

kripton commented Apr 2, 2024

cbix commented Mar 30, 2024 •

edited

Loading

cbix Mar 30, 2024 •

edited

Loading

kripton left a comment •

edited

Loading