Bluetooth: audio: has: Add non-volatile settings

[MariuszSkamra]

This adds non-volatile settings for the HAS Server. Those are needed to
restore the client awareness of preset list entries exposed by the
server. Based on the settings, the implementation determines how to
notify the client about the HAS related characteristic value changes.

Depends on: #61980

[MariuszSkamra]

Open questions:

1. Should we store the last notified Active Preset Index? We could avoid sending the active index notification to devices that already know this index. e.g.:

• Client is connected and know the active index of 1.
• Client disconnects
• The active index have changed from 1 to 2
• The active index have changed back from 2 to 1
• Client connects and receives no active index notification because even if it changed over time, the current one is the one that Client was aware of.

2. Same question applies to Hearing Aid Features value. Should we remember the last notified value for each client?

3. As a optimization, should we avoid sending the Preset Changed notifications to Clients that did not performed Read Presets procedure? The motivation is, if the client does not know the list of the presets (e.g. it's simple remote controller that does Next/Previous preset operations), the preset list updates are useless for this device.

Those questions relate to this PR, because if the answer is "Yes" to any of those questions then more information will be needed to be stored in NVM.

[Thalley]

1. Should we store the last notified Active Preset Index? We could avoid sending the active index notification to devices that already know this index. e.g.:

I guess that depends on the semantic meaning of what "value has changed" is.
If a value changes from 1 to 2 to 1, has it changed?
The simple solution is to say yes, and then notify the value to the client, regardless of what the value was when the client disconnected.
The optimized solution (in terms of radio, at the cost of more flash) is to say no, and then not sending the notification.

I'm mostly in favor of saying yes it has changed, and no we should not store last value.

2. Same question applies to Hearing Aid Features value. Should we remember the last notified value for each client?

Same answer

3. As a optimization, should we avoid sending the Preset Changed notifications to Clients that did not performed Read Presets procedure? The motivation is, if the client does not know the list of the presets (e.g. it's simple remote controller that does Next/Previous preset operations), the preset list updates are useless for this device.

Isn't this breaking spec compliancy? If a client subscribes to notifications, don't we have to send them regardless of whether they read the previous value?

[Thalley]

Overall what this PR suggests, is that the services themselves will be responsible for storing and retrieving data from flash (similar to how GAP and GATT does). I don't think that is a bad idea, compared to providing the application the tools to do the same.

One thing we need to consider is that the structure of these settings may be fairly complex for some of the larger services, and they can easily be error prone, so we need to find a way to catch any errors that may be introduced with new updates, i.e. we need to be able to test these in CI

[Thalley]

We don't really have documentation for the internal bt_settings APIs; when setting id=0 does that mean delete for any IDs, or specifically for BT_ID_DEFAULT (0)?

[MariuszSkamra]

I believe it deletes the one with the ID that was provided when the entry was added (0 in this case).

[Thalley]

Looks like id == 0 is a special case:

int bt_settings_delete(const char *key, uint8_t id, const bt_addr_le_t *addr)
{
	int err;
	char id_str[4];
	char key_str[BT_SETTINGS_KEY_MAX];

	if (addr) {
		if (id) {
			u8_to_dec(id_str, sizeof(id_str), id);
		}

		bt_settings_encode_key(key_str, sizeof(key_str), key, addr, (id ? id_str : NULL));

[Thalley]

Not necessarily something that needs to be fixed in this PR, but I think we should figure out and understand how the 0 ID value here is used in the bt_settings, and preferably also replace all the literal constant values used by the stack with a #define if it has special meaning.

[Thalley]

@jhedberg Looks like you and @Vudentz have been involved with the bt_settings, right?

[Thalley]

@jhedberg Do you know the intention of the id parameter in bt_settings?

[fredrikdanebjer]

Open questions:

1. Should we store the last notified Active Preset Index? We could avoid sending the active index notification to devices that already know this index. e.g.:

• Client is connected and know the active index of 1.
• Client disconnects
• The active index have changed from 1 to 2
• The active index have changed back from 2 to 1
• Client connects and receives no active index notification because even if it changed over time, the current one is the one that Client was aware of.

2. Same question applies to Hearing Aid Features value. Should we remember the last notified value for each client?
3. As a optimization, should we avoid sending the Preset Changed notifications to Clients that did not performed Read Presets procedure? The motivation is, if the client does not know the list of the presets (e.g. it's simple remote controller that does Next/Previous preset operations), the preset list updates are useless for this device.

Those questions relate to this PR, because if the answer is "Yes" to any of those questions then more information will be needed to be stored in NVM.

As I mentioned on host-meeting, as I see this, for all mentioned cases in worst case scenarios we will always have to send all notifications anyway. This means that we actually can't save memory, but instead just add usage in NVM.
I understand that we could save some radio time, but I'm not confident enough to comment on how big of a deal this is in this case. If it's only a minor negible improvement I think its worth thinking about the extra layer of complexity this feature adds for future maintainers and other interested people in the code base.

I would have to say that I'm tilting towards saying no, we don't want these optimizations.

[kruithofa]

Open questions:

1. Should we store the last notified Active Preset Index? We could avoid sending the active index notification to devices that already know this index. e.g.:

I think it is preferable to send the notification, which gives simpler code.

2. Same question applies to Hearing Aid Features value. Should we remember the last notified value for each client?

Same as for 1.

3. As a optimization, should we avoid sending the Preset Changed notifications to Clients that did not performed Read Presets procedure? The motivation is, if the client does not know the list of the presets (e.g. it's simple remote controller that does Next/Previous preset operations), the preset list updates are useless for this device.

IMO the client subscribed to notifications, so we need to send the notification.

[MariuszSkamra]

• Rebased
• Added __packed to struct client_context_store

[Thalley]

PR LGTM but I'd like to get a resolution of the ID for bt_settings.

The literal value 0 should IMO not be used.

If it refers to the default ID, BT_ID_DEFAULT should be used.
If it refers to some magical number in BT_SETTINGS then we need to expose that as a value and use that.