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

[async] InterfaceAdded signal not emitted by generated aserver? #86

Open
vsytch opened this issue Oct 27, 2023 · 3 comments
Open

[async] InterfaceAdded signal not emitted by generated aserver? #86

vsytch opened this issue Oct 27, 2023 · 3 comments
Assignees

Comments

@vsytch
Copy link
Contributor

vsytch commented Oct 27, 2023

I've been having trouble with ObjectMapper not automatically discovering paths with interfaces added using sdbusplus/async. Here is example code:


using namespace sdbusplus::aserver::xyz::openbmc_project;

struct DBusSensor : public association::Definitions<DBusSensor>,
                    public sensor::Value<DBusSensor>,
                    public state::decorator::Availability<DBusSensor>,
                    public state::decorator::OperationalStatus<DBusSensor> {
  DBusSensor(sdbusplus::async::context& ctx, const char* sensor_path,
             const char* associated_path)
      : association::Definitions<DBusSensor>(ctx, sensor_path),
        sensor::Value<DBusSensor>(ctx, sensor_path),
        state::decorator::Availability<DBusSensor>(ctx, sensor_path),
        state::decorator::OperationalStatus<DBusSensor>(ctx, sensor_path) {
    value(0);
    min_value(std::numeric_limits<int8_t>::min());
    max_value(std::numeric_limits<int8_t>::max());
    functional(true);
    available(true);

    using value_type = decltype(associations());
    associations(value_type{{"chassis", "all_sensors", associated_path}});
  }
};

Am I doing something wrong? Do I have to manually call emit_added()? I couldn't find it being invoked anywhere. This does feel like a regression compared to the asio API.

As a workaround I hacked sdbusplus/server/object.hpp to be compatible async::context:

$ diff include/sdbusplus/async/aobject.hpp include/sdbusplus/server/object.hpp
11c11
< namespace async
---
> namespace server
76c76
<     object(context& ctx, const char* path,
---
>     object(bus_t& bus, const char* path,
78,79c78,79
<         details::compose<Args...>(ctx, path),
<         __sdbusplus_server_object_bus(get_busp(ctx.get_bus()), ctx.get_bus().getInterface()),
---
>         details::compose<Args...>(bus, path),
>         __sdbusplus_server_object_bus(get_busp(bus), bus.getInterface()),
81c81
<         __sdbusplus_server_object_intf(ctx.get_bus().getInterface())
---
>         __sdbusplus_server_object_intf(bus.getInterface())
87c87
<     ~object()
---
>     ~object() override
183,184c183,184
<     compose(context& ctx, const char* path) :
<         compose_inherit<Args>::type(ctx, path)... {};
---
>     compose(bus_t& bus, const char* path) :
>         compose_inherit<Args>::type(bus, path)... {}

And my new DBusSensor is now:

template<typename Instance>
using DBusSensorInterfaces = sdbusplus::async::object_t<
  sdbusplus::aserver::xyz::openbmc_project::association::Definitions<Instance>,
  sdbusplus::aserver::xyz::openbmc_project::sensor::Value<Instance>,
  sdbusplus::aserver::xyz::openbmc_project::state::decorator::Availability<Instance>,
  sdbusplus::aserver::xyz::openbmc_project::state::decorator::OperationalStatus<Instance>
>;

struct DBusSensor : public DBusSensorInterfaces<DBusSensor> {
  DBusSensor(sdbusplus::async::context& ctx, const char* sensor_path,
             const char* associated_path)
      : DBusSensorInterfaces<DBusSensor>(ctx, sensor_path, action::emit_interface_added) {
    value(0);
    min_value(std::numeric_limits<int8_t>::min());
    max_value(std::numeric_limits<int8_t>::max());
    functional(true);
    available(true);

    using value_type = decltype(associations());
    associations(value_type{{"chassis", "all_sensors", associated_path}});
  }
};
@williamspatrick
Copy link
Member

Correct, the current code for async does not do any automatic signal processing. I've had some complaints about how it was done in the previously sync implementation and I didn't want to put too much in place until I had some time to think more deeply about it. I don't want to put something in place and get tied to a method that doesn't really work.

What I'm thinking I'll likely need to do is to add some information to the async-context that keeps track if a service name has been requested. We don't actually want signals to be sent prior to a service name being allocated and they deceptively get dropped anyhow.

@vsytch
Copy link
Contributor Author

vsytch commented Nov 1, 2023

Understood, thank you. Explicit signals are OK, I'll make a note of this and let others know.

@vsytch vsytch closed this as completed Nov 1, 2023
@williamspatrick
Copy link
Member

Just to be clear, this will probably get resolved in the next few months. We can leave this open if you want something to track the improvement against.

@williamspatrick williamspatrick self-assigned this Nov 2, 2023
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

No branches or pull requests

2 participants