zephyr/tests/bluetooth/hci
Rubin Gerritsen bfc0cdc905 Bluetooth: Conditionally print out HCI error codes as strings
When developing Bluetooth applications, you typically run into
some errors. If you are an experienced Bluetooth developer,
you would typically have an HCI error lookup table in your memory.
Others might not.

This commit utilizes defines CONFIG_BT_DEBUG_HCI_ERR_TO_STR
and utilizes bt_hci_err_to_str() to print out HCI error strings
when enabled to improve the user experience.

Several alternatives where considered. This approach was chosen
as it had the best balance between readability, code size, and
implementation complexity.

The alternatives are listed below as a reference.

1. Macro defined format specifier:

```c
  #define HCI_ERR_FMT "%s"
  #define BT_HCI_ERR_TO_STR(err) (err)
  #define HCI_ERR_FMT "%d"
  #define BT_HCI_ERR_TO_STR(err) bt_hci_err_to_str((err))

LOG_INF("The event contained " HCI_ERR_FMT " as status",
	BT_HCI_ERR_TO_STR(err));
```
Advantage: Space efficient: Code size does not increase
Disadvantage: Code becomes hard to read

2. Format specifier to always include both integer and string:

```c
static inline const char bt_hci_err_to_str(err)
{
	return "";
}

LOG_INF("The event contained %s(0x%02x) as status",
	bt_hci_err_to_str(err), err);
```

Advantage: Simple to use, implement, and read,
Disadvantage: Increases code size when CONFIG_BT_DEBUG_HCI_ERR_TO_STR
is disabled. The compiler seems unable to optimize away the unused
format specifier. Note: The size increase is only present when
logging is enabled.

3. Always print as string, allocate a stack variable when printing:

```c
const char *bt_hci_err_to_str(char *dst, size_t dst_size, uint8_t err)
{
  snprintf(dst, dst_size, 0x%02x, err);
  return dst;
}

LOG_INF("The event contained %s as status", BT_HCI_ERR_TO_STR(err));
```

Advantage: Very easy to read.
Disadvantage: Printing error codes becomes slow as it involves calling
snprint.

4. Implement a custom printf specifier, for example E.

   This requires a global CONFIG_ERR_AS_STR as I assume we cannot have
   one specifier for each type of error code.
   Also, I assume we cannot start adding specifiers for each subsystem.

```c
  #define BT_HCI_ERR_TO_STR(err) (err)
  #define BT_HCI_ERR_TO_STR(err) bt_hci_err_to_str((err))

LOG_INF("The event contained %E as status", BT_HCI_ERR_TO_STR(err));
```

Advantage: Both efficient code and readable code.
Disadvantage: This requires a global CONFIG_ERR_AS_STR as I assume
we cannot have one specifier for each type of error code.
Also, I assume we cannot start adding specifiers for each subsystem.
That is, this approach is hard to implement correctly in a scalable
way.

Signed-off-by: Rubin Gerritsen <rubin.gerritsen@nordicsemi.no>
2024-07-30 18:25:56 +01:00
..
src
CMakeLists.txt
prj.conf
testcase.yaml