sensorfw: Fixes from LuneOS

[Herrie82]

insertMulti has been deprecated as per https://doc.qt.io/qt-6/qmultimap-obsolete.html#insertMulti

Use QMultiMap insert instead.

LuneOS uses a hybrid between autohybris and hybris, add a CONFIG option for LuneOS simply.

Signed-off-by: Herman van Hazendonk github.com@herrie.org

[mlehtima]

Wondering if adding autobinder option would be cleaner than a distro-specific option? That would go to config.tests folder and would be very similar to the autohybris option there.

[Herrie82]

Wondering if adding autobinder option would be cleaner than a distro-specific option? That would go to config.tests folder and would be very similar to the autohybris option there.

We carried this patch for a long time at our end (we're using autohybris for Halium based targets):
webOS-ports@6d8c671

This looks cleaner to me vs the patch, because it's configurable, but open for other suggestions of course! Not really sure how the config.tests variant would work.

[mlehtima]

We carried this patch for a long time at our end (we're using autohybris for Halium based targets): webOS-ports@6d8c671

This looks cleaner to me vs the patch, because it's configurable, but open for other suggestions of course! Not really sure how the config.tests variant would work.

I'm a bit confused what the build issue is. That config_hybris in common-config.pri should already handle adding android linking when using autohybris which suggests only the "LIBS += -lhybris-common" is the problem? And that in my opinion probably should go from core/hybris.pro to common-config.pri next to the two places which have "PKGCONFIG += android-headers libhardware". After those changes the !contains(CONFIG,binder) part in hybris.pro is not needed anymore, does this sound like a clean way to you?

[Herrie82]

We carried this patch for a long time at our end (we're using autohybris for Halium based targets): webOS-ports@6d8c671
This looks cleaner to me vs the patch, because it's configurable, but open for other suggestions of course! Not really sure how the config.tests variant would work.

I'm a bit confused what the build issue is. That config_hybris in common-config.pri should already handle adding android linking when using autohybris which suggests only the "LIBS += -lhybris-common" is the problem? And that in my opinion probably should go from core/hybris.pro to common-config.pri next to the two places which have "PKGCONFIG += android-headers libhardware". After those changes the !contains(CONFIG,binder) part in hybris.pro is not needed anymore, does this sound like a clean way to you?

If it would work yes, but with these changes I don't get sensors to work somehow (we had same issue before...)

This is my log:
https://bpa.st/PTEQ

When I install the new sensorfw IPK with luneos CONFIG option as per this PR, it does behave and things show up in Messwerk: (see line 88 onwards):

https://bpa.st/FNSQ

Looping in @Tofee

[mlehtima]

Can you show which changes you had when you got those logs?

[Herrie82]

Can you show which changes you had when you got those logs?

The changes you suggested, or I must have misread what you were suggesting?

Herrie82@8a83c45

BTW at our end we're building with:

"CONFIG+=autohybris "
"CONFIG+=binder "

On Android/Halium 9 based targets.

[Tofee]

The issue we have, if we don't have this kind of patch, is that sensorfw seems to use libhybris incorrectly. I can't recall the precise root cause though, I'd have to investigate a bit.

[mlehtima]

Is the autohybris test really working for you? The issue suggests maybe not, that config_hybris in common-config.pri should be adding the needed linking when autohybris detects that hybris exists in build env. If it really is detecting hybris then there is something in how Qt project files work that I don't understand.

[Tofee]

I've looked at it more carefully: our original problem was that we couldn't use binder and autohybris together.
However, the fix I used at first was really not minimal, and now that I understand the issue better I've come up with a better proposal, i.e. replace the whole PR with:

diff --git a/common-config.pri b/common-config.pri
index 1ac908b..a4736ca 100644
--- a/common-config.pri
+++ b/common-config.pri
@@ -46,5 +46,10 @@ contains(CONFIG,hybris) {
 
 config_hybris {
     CONFIG += link_pkgconfig
-    PKGCONFIG += android-headers libhardware
+    contains(CONFIG,binder) {
+        DEFINES += USE_BINDER=1
+        PKGCONFIG += libgbinder libglibutil gobject-2.0 glib-2.0
+    } else {
+        PKGCONFIG += android-headers libhardware
+    }
 }

@Herrie82 @mlehtima what do you think? It works at least on our Tissot A1 target phone.

[mlehtima]

Ok,now I understand, that looks quite nice. I still wonder if my suggestion could be made also, not necessary but in a way the current code is a bit complicated because it has hybris linking in two files when all could be just in common-config.pri.

[Herrie82]

If it works on Tissot it will work on the others too. So LGTM

[mlehtima]

Although maybe I need to think about the linking a bit more so that we don't link hybris too much. So probably for now your simple change should be better.

[mlehtima]

Also unrelated note, I have been thinking of moving away from qmake build system in sensorfw, probably to meson but also cmake is an option.

[Tofee]

@mlehtima FWIW I sketched a commit (on top of #22) where linking is simplified a bit, when not using binder: Tofee@989c004

However it's not even build-tested, as we use binder in all our sensorfw builds...

[mlehtima]

I looked at the code again and the code is not that logical. Using autohybris (which requires android headers to be present) and binder (which doesn't need android headers) is a bit odd. The whole idea of having binder in a way as a child of hybris is not really that correct. That is probably just some mistake from the past and not sure if it would make your builds easier or more difficult if we separate those completely.

[Herrie82]

Also unrelated note, I have been thinking of moving away from qmake build system in sensorfw, probably to meson but also cmake is an option.

Either would work from our side. We can build all ;)

[Herrie82]

Seeing the other PR was merged, closing this one