On Tue, 2025-05-20 at 10:36 -0400, Luiz Augusto von Dentz wrote:
> Hi Bastien,
>
> On Tue, May 20, 2025 at 9:27 AM Bastien Nocera <hadess@xxxxxxxxxx>
> wrote:
> >
> > ---
> > src/device.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/device.c b/src/device.c
> > index 0797e5ff5bb8..d1023f762474 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -1922,9 +1922,9 @@ void device_request_disconnect(struct
> > btd_device *device, DBusMessage *msg)
> > DBusMessage *reply;
> >
> > if (device->bonding_status ==
> > MGMT_STATUS_AUTH_FAILED)
> > - err_str = ERR_BREDR_CONN_KEY_MISSING;
> > + err_str = ":" ERR_BREDR_CONN_KEY_MISSING
> > ":Link key missing";
> > else
> > - err_str = ERR_BREDR_CONN_CANCELED;
> > + err_str = ":" ERR_BREDR_CONN_CANCELED
> > ":Connection canceled";
> > reply = btd_error_failed(device->connect, err_str);
> > g_dbus_send_message(dbus_conn, reply);
> > dbus_message_unref(device->connect);
> > @@ -2545,7 +2545,8 @@ static DBusMessage *connect_profiles(struct
> > btd_device *dev, uint8_t bdaddr_type
> >
> > if (!btd_adapter_get_powered(dev->adapter)) {
> > return btd_error_not_ready_str(msg,
> > -
> > ERR_BREDR_CONN_ADAPTER_NOT_POWERED);
> > + ":"
> > ERR_BREDR_CONN_ADAPTER_NOT_POWERED
> > + ":Adapter not powered");
> > }
> >
> > btd_device_set_temporary(dev, false);
> > @@ -2564,7 +2565,8 @@ static DBusMessage *connect_profiles(struct
> > btd_device *dev, uint8_t bdaddr_type
> > return
> > dbus_message_new_method_return(msg);
> > } else {
> > return
> > btd_error_not_available_str(msg,
> > -
> > ERR_BREDR_CONN_PROFILE_UNAVAILABLE);
> > + ":"
> > ERR_BREDR_CONN_PROFILE_UNAVAILABLE ":"
> > + "Exhausted the list of
> > BR/EDR profiles to connect to");
> > }
> > }
> >
> > --
> > 2.49.0
>
> I was afraid we would need to change these string again, also if we
> need to encode more than human readable errors on on the message
> something is already quite wrong, so I wonder if we shall instead
> expand the error code itself e.g.:
>
> DBusMessage *btd_error_profile_unavailable_str(DBusMessage *msg,
> const
> char *str)
> {
> return g_dbus_create_error(msg, ERROR_INTERFACE
> ".ProfileUnavailable", str);
> }
>
> That in my opinion uses the error reply better since we can isolate
> what is considered a fixed string, and therefore part of the stable
> API, and what could be considered a free format which is the error
> message.
So, for this example, a new error code
org.bluez.Error.ProfileUnavailable to be added to
doc/org.bluez.Device.rst right?
Or would org.bluez.Error.Failed.ProfileUnavailable make it easier for
front-ends to parse? gnome-bluetooth doesn't (currently) have any code
to differentiate those, so it's not a problem on my side.
What about the human-readable strings, would you want bluetoothctl to
translate the error code to something human-readable, or should we have
a human-readable message right in src/device.c in this case.
In short, does this work for you?
DBusMessage *btd_error_failed_profile_unavailable(DBusMessage *msg)
{
return g_dbus_create_error(msg, ERROR_INTERFACE
".Failed.ProfileUnavailable", "Exhausted the list of
BR/EDR profiles to connect to");
}
>
> >
>
