openthread / ot-br-posix

OpenThread Border Router, a Thread border router for POSIX-based platforms.
https://openthread.io/
BSD 3-Clause "New" or "Revised" License
419 stars 231 forks source link

[Dbus] Set and GetAll methods throwing Error io.openthread.Error.NotFound: io.openthread.Error.NotFound error #2133

Open gadekula1 opened 10 months ago

gadekula1 commented 10 months ago

Dbus set and GetAll methods throwing error.

Details:

DBUS_PROPERTY_SET_METHOD "Set" DBUS_PROPERTY_GET_ALL_METHOD "GetAll"

Introspect for GetAll:

dbus-send command : dbus-send --system --print-reply --dest=io.openthread.BorderRouter.wpan0 /io/openthread/BorderRouter/wpan0 org.free desktop.DBus.Properties.GetAll string:"io.openthread.BorderRouter" Error io.openthread.Error.NotFound: io.openthread.Error.NotFound

dbus-send --system --print-reply --dest=io.openthread.BorderRouter.wpan0 /io/openthread/BorderRouter/wpan0 org.free desktop.DBus.Properties.GetAll string:io.openthread.BorderRouter Error io.openthread.Error.NotFound: io.openthread.Error.NotFound

Introspect for set method:

dbus-send command

dbus-send --system --print-reply --dest=io.openthread.BorderRouter.wpan0 /io/openthread/BorderRouter/wpan0 org.free desktop.DBus.Properties.Set string:io.openthread.BorderRouter string:RadioRegion variant:string:"1234" Error io.openthread.Error.InvalidArgs: io.openthread.Error.InvalidArgs

dbus-send --system --print-reply --dest=io.openthread.BorderRouter.wpan0 /io/openthread/BorderRouter/wpan0 org.free desktop.DBus.Properties.Set string:io.openthread.BorderRouter string:"RadioRegion" variant:string:"1234" Error io.openthread.Error.InvalidArgs: io.openthread.Error.InvalidArgs

superwhd commented 10 months ago

I think your Set method failed because you didn't pass in a valid region code. Below command works for me.

dbus-send --system --print-reply --dest=io.openthread.BorderRouter.wpan0 /io/openthread/BorderRouter/wpan0 org.freedesktop.DBus.Properties.Set string:"io.openthread.BorderRouter" string:"RadioRegion" variant:string:"US"
superwhd commented 10 months ago

For the GetAll method, it failed due to an error when getting one of the unimplemented properties. When a feature is disabled at build time, its 'GetPropertyHandler' is still registered but it throws an OT_NOT_IMPLEMENTED error which results in the failure of GetAll.

@morningboata Shall we let GetAll tolerate OT_NOT_IMPLEMENTED error? See https://github.com/openthread/ot-br-posix/blob/29cea6ac3d60862a2e2082f72b2ddb238e97d046/src/dbus/server/dbus_object.cpp#L207

morningboata commented 9 months ago

It seems that once the compile time flag is defined, the runtime behavior is well-defined, so that we can fix the property's compile time registration issue instead of tolerate runtime OT_NOT_IMPLEMENTED error? That is:

  1. print detail property name (p.first) and 'not implemented' error around

SuccessOrExit(error = p.second(dictEntryIter));

  1. fix the error by stopping the property's registration if it is not available, similar to https://github.com/openthread/ot-br-posix/blob/main/src/dbus/server/dbus_thread_object.cpp#L209C3-L209C3

How do you think?

superwhd commented 9 months ago

Either one seems good to be. The first approach seems simpler to implement because it's feature-agnostic.

morningboata commented 2 months ago

Agree, and the first approach is more preferred due to its simplicity. Anything you need from here?

caipiblack commented 1 month ago

dbus-send --system --print-reply --dest=io.openthread.BorderRouter.wpan0 /io/openthread/BorderRouter/wpan0 org.free desktop.DBus.Properties.GetAll string:"io.openthread.BorderRouter"

Hello, I am not sure to understand.

In my system we have the CHANNEL MONITOR disabled (due to siliconlabs multipan stuff).

This is the code in dbus_thread_object.cpp:

otbrError DBusThreadObject::Init(void)
{
...
#if OPENTHREAD_CONFIG_CHANNEL_MONITOR_ENABLE
    RegisterGetPropertyHandler(OTBR_DBUS_THREAD_INTERFACE, OTBR_DBUS_PROPERTY_CHANNEL_MONITOR_SAMPLE_COUNT,
                               std::bind(&DBusThreadObject::GetChannelMonitorSampleCountHandler, this, _1));
    RegisterGetPropertyHandler(OTBR_DBUS_THREAD_INTERFACE, OTBR_DBUS_PROPERTY_CHANNEL_MONITOR_ALL_CHANNEL_QUALITIES,
                               std::bind(&DBusThreadObject::GetChannelMonitorAllChannelQualities, this, _1));
#endif
...
}

otError DBusThreadObject::GetChannelMonitorAllChannelQualities(DBusMessageIter &aIter)
{
#if OPENTHREAD_CONFIG_CHANNEL_MONITOR_ENABLE
    auto                        threadHelper = mNcp->GetThreadHelper();
    otError                     error        = OT_ERROR_NONE;
    uint32_t                    channelMask  = otLinkGetSupportedChannelMask(threadHelper->GetInstance());
    constexpr uint8_t           kNumChannels = sizeof(channelMask) * 8; // 8 bit per byte
    std::vector<ChannelQuality> quality;

    for (uint8_t i = 0; i < kNumChannels; i++)
    {
        if (channelMask & (1U << i))
        {
            uint16_t occupancy = otChannelMonitorGetChannelOccupancy(threadHelper->GetInstance(), i);

            quality.emplace_back(ChannelQuality{i, occupancy});
        }
    }

    VerifyOrExit(DBusMessageEncodeToVariant(&aIter, quality) == OTBR_ERROR_NONE, error = OT_ERROR_INVALID_ARGS);

exit:
    return error;
#else  // OPENTHREAD_CONFIG_CHANNEL_MONITOR_ENABLE
    OTBR_UNUSED_VARIABLE(aIter);
    return OT_ERROR_NOT_IMPLEMENTED;
#endif // OPENTHREAD_CONFIG_CHANNEL_MONITOR_ENABLE
}

So as the code is, the function is not supposed to be registered.

Or maybe in my version this has been fixed and my issue is not here.

I am using the OpenThread version from Gecko 4.3.3, but most generally the most recent code seems to be the same in this file around these functions.

superwhd commented 1 month ago

@bukepo Do you have any idea why we decided to register the method when the feature is not unvailable?

caipiblack commented 1 month ago

@bukepo Do you have any idea why we decided to register the method when the feature is not unvailable?

Do you think the problem is channel monitor ? I don’t think because it is not supposed to be registered if you look the code.

When OPENTHREAD_CONFIG_CHANNEL_MONITOR_ENABLE is false, this is not registered.

Am i missing something ?

superwhd commented 1 month ago

Unfortunately I cannot recall what property caused my previous error. According to the code, channel monitor doesn't seem to be the culprit because its DBUS method is not registered when the feature is disabled.

However, there are other properties which are still registered even when the feature is disabled at build time. See the return OT_ERROR_NOT_IMPLEMENTED; statements in https://github.com/openthread/ot-br-posix/blob/main/src/dbus/server/dbus_thread_object_rcp.cpp.