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

interface should define either direction or add annotation for both directions #14

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

jmlich
Copy link
Contributor

@jmlich jmlich commented Jul 24, 2023

The Qt6 qdbusxml2cpp seems to be more strict about annotations and complains on missing annotation for both directions.

$ /usr/lib/qt6/bin/qdbusxml2cpp -N -c QUsbModedInterface -p usb_moded_interface.h: /usr/include/usb-moded/com.meego.usb_moded.xml
dbus.parser: Invalid D-BUS object path '//com/meego/usb_moded' found while parsing introspection
qdbusxml2cpp: Got unknown type `a{sv}' processing '/usr/include/usb-moded/com.meego.usb_moded.xml'
You should add <annotation name="org.qtproject.QtDBus.QtTypeName.Out0" value="<type>"/> to the XML description for 'config'

Related part of the xml looks like this:

    <signal name="sig_usb_taget_mode_config_ind">
      <arg name="config" type="a{sv}" />
      <annotation name="org.qtproject.QtDBus.QtTypeName.In0" value="QVariantMap"/>
    </signal>

The fix should either add direction like this:

    <signal name="sig_usb_taget_mode_config_ind">
      <arg name="config" type="a{sv}" direction="in"/>
      <annotation name="org.qtproject.QtDBus.QtTypeName.In0" value="QVariantMap"/>
    </signal>

Either add annotation for both directions.

    <signal name="sig_usb_taget_mode_config_ind">
      <arg name="config" type="a{sv}" />
      <annotation name="org.qtproject.QtDBus.QtTypeName.In0" value="QVariantMap"/>
      <annotation name="org.qtproject.QtDBus.QtTypeName.Out0" value="QVariantMap"/>
    </signal>

My best guess is that annotations for both directions will not break anything, but I am not familiar with usb-moded code enough to be 100% sure.

@jmlich jmlich force-pushed the master branch 2 times, most recently from ec4a01f to 654576e Compare July 24, 2023 17:25
jmlich added a commit to nemomobile-ux/nemo-packaging that referenced this pull request Jul 24, 2023
@jmlich jmlich mentioned this pull request Jul 24, 2023
52 tasks
@mlehtima mlehtima requested a review from spiiroin July 24, 2023 21:28
@Thaodan
Copy link
Contributor

Thaodan commented Aug 7, 2023

Similar to sailfishos/libconnman-qt#20. Should be safe.

@pvuorela
Copy link
Contributor

Using my desktop Qt 6.6.0 qdbus tool I'm not getting any complaints. And then as mentioned signals should be already implicitly 'out' parameters.

For In0 came across this https://techbase.kde.org/Development/Tutorials/D-Bus/CustomTypes -- Sounds like that annotation can be used interchangeably with Out0.

So I'm not sure is there really anything needing changes here.

@spiiroin
Copy link
Contributor

So I'm not sure is there really anything needing changes here.

There is the actual fix part i.e. changing "In0" -> "Out0" removes the compilation time warning -> LGTM.

Then there is unnecessary addition of "direction=out". Which is not wrong. But it also makes that one signal definition differ from all the other signals. So for the sake of consistency -> I'd rather would not to have this.

Copy link
Contributor

@spiiroin spiiroin left a comment

Choose a reason for hiding this comment

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

Fixes the reported problem.
Still has a minor cosmetic issue, but that is not a show stopper. Can be merged.

@spiiroin spiiroin merged commit 5ed302d into sailfishos:master Feb 7, 2024
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.

4 participants