Re: [PATCH BlueZ v3 4/4] device: Add user-readable messages for a number of errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

>


-- 
Luiz Augusto von Dentz





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux