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

Fix for duplicated interfaces in the introspection data; missing DBus annotations fixed #143

Merged

Conversation

mk868
Copy link
Contributor

@mk868 mk868 commented May 9, 2021

I would like to explain the problem with the duplicated interfaces in the introspection data with an example, let's suppose we have interfaces:

@DBusInterfaceName("com.example.HalModule")
@DBusProperty(name = "Name", type = String.class, access = Access.READ)
@DBusProperty(name = "SerialNumber", type = String.class)
public interface HalModule extends DBusInterface {

  @DeprecatedOnDBus
  @IntrospectionDescription("Test description")
  void test();
}
@DBusInterfaceName("com.example.PowerController")
@DBusProperty(name = "UnitStatus", type = String.class, access = Access.READ)
@DBusProperty(name = "Load", type = Double.class, access = Access.READ)
public interface PowerController extends DBusInterface {

}

Both interfaces have the @DBusProperty annotations - so we can expect an object that implements HalModule or PowerController to implement Properties as well.
Forcing the Properties implementation can be accomplished with extending it by HalModule and PowerController - maybe it was not the original assumption of interfaces behavior but it allows to simplify the application logic.
So the enriched interfaces will look like this:

@DBusInterfaceName("com.example.HalModule")
@DBusProperty(name = "Name", type = String.class, access = Access.READ)
@DBusProperty(name = "SerialNumber", type = String.class)
public interface HalModule extends DBusInterface, Properties {

  @DeprecatedOnDBus
  @IntrospectionDescription("Test description")
  void test();
}
@DBusInterfaceName("com.example.PowerController")
@DBusProperty(name = "UnitStatus", type = String.class, access = Access.READ)
@DBusProperty(name = "Load", type = Double.class, access = Access.READ)
public interface PowerController extends DBusInterface, Properties {

}

And DBus object class:

public class PowerModuleObject implements PowerController, HalModule {
  //...
}

Currently this logic results in duplicated Properties interface in introspection data:
dbus_pr1

This PR:

  • fixes the problem with duplicated interfaces in the introspection data
  • adds a new DBusNamingUtil class which will be helpful for internal and external use
  • fixes a bug with missing DBus annotations (eg @DeprecatedOnDBus, @IntrospectionDescription) in the introspection data

@hypfvieh
Copy link
Owner

As always, great work, thanks for the PR

@hypfvieh hypfvieh merged commit c801402 into hypfvieh:master May 10, 2021
hypfvieh added a commit that referenced this pull request Sep 14, 2021
Using HashSet and putting methods to a map without checking if this is a
duplicate will cause random failures in TestAll test.

TestAll has a test case which will override the 'Introspect' method with
some bogus method.

ExportedObject will create the Introspection data and will maintain a
list of methods which can be called on the containing object.
It also adds Peer and Introspectable to every exported object.

When a method of Introspectable (or Peer) is overridden by another
interface, HashSet will cause random failures because the export order
is not retained. Using LinkedHashSet here will fix the order issue,
because the methods of the given class/interface is added first, and the
additional methods pulled in from Introspection and Peer interface is
added last.

It is also required to check if a method is already part of the
'methods' map, otherwise the 'good' method will be overridden by the
'bad' method because without check the later always wins.
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.

None yet

2 participants