Just to make sure I understand things correctly: the parameter to updateServices should have a structure similar to the setServices one but only value (and maybe maxLen?) should be taken into account (I don't think reconfiguring services should be covered by it since calling setServices again serves that purpose already)

When compared to the simple write example you gave on the forum, updateServices has the advantage that several values can be updated "atomically". This could be convenient if these values are somehow correlated.
The downside is that the argument has to be correctly documented in order not to confuse users who are familiar with setServices (it will be a similar structure but treated differently).

A corollary is that maybe updateServices should only allow existing services and characteristics to be updated. If so, should we drop the unwanted entries of throw an error?
The simple write avoids all these issues since only existing values can be accessed but we would loose the possibility to atomically update multiple values.

Yes, that all sounds good.

I'd iterate through the structure in a similar way to setServices, but then if you can't find the relevant UUID just throw an error.

For now it would be easier to just ignore any non-'value' entries, and it has the benefit that a user could keep the structure they had used previously, update the value element, and re-use it.

Good idea.
I guess I have everything I need to give it a try :)

I just pushed some changes that fix issues related to setServices. It's work doing a pull because it might save you some problems when debugging :)

I just pulled the changes thanks!

Did you ever have any luck with this, or should I push on with it?

Sorry, I should have touched base before I went on holiday. I am currently away until the end of the month and I don't have my dev boards with me. I still had a few issues with the code I had written before I left but, for the most part, the call to sd_ble_gatts_hvx seemed to work. I'll be back at the office in October.

I think I am going to need a little help here... I pushed some working code on my repo: master...ddm:issue-915

The remaining thing to implement is getting the proper characteristic handle (cf. // TODO FINISH Get the handle). I wonder if the nRF SDK has a way to retrieve a characteristic handle based on the UUID at hand. Maybe @mjdietzx has more information?

Otherwise, setServices will have to be modified to store a UUID → handle map of some kind...

Thanks for that!

All I can see is sd_ble_gatts_attr_get - you might be able to iterate through the handles checking their UUIDs? Realistically there should be <10 handles, so iteration wouldn't be the end of the world.

From what I understand sd_ble_gatts_attr_get is handle → UUID but maybe sd_ble_gatts_initial_user_handle_get? But how would I iterate over the other ones?

Yes, you'd have to try out all the handles (they're sequential I think?) until you got an error message, checking each one until you got the right UUID. Otherwise, yes - I guess it'd be a matter of storing the mapping somewhere.

Hopefully this is the correct way to do it: #933

Keeping the mapping seems unnecessary at this point unless the calls to sd_ble_gatts_initial_user_handle_get and sd_ble_gatts_attr_get are really too expensive for some reason.

I think #933 mostly addresses the issue but indication (aka. notification+ACK) support is still missing. Adding indicate: true via NRF.setServices and modifying NRF.updateServices accordingly could fix this. Should I submit another PR linked to this issue or would a new issue be more appropriate ?

Also, I should test tomorrow if updates without notification work properly.

Yes, a PR for that would be great - I'm fine with just mentioning this issue in it. Would that involve calling some JS code when the ACK was received as well?

Just one extra question - while looking at the API, did you find a nice way of removing attributes? As it is, setServices can only be called once - which is far from ideal. I have a feeling I might need to call something that removes everything, and then re-add it all (including the nordic uart, if needed)

I'll check if an 'onAck' callback is possible to implement, but it is not strictly necessary to act on the ACK from JS.

I haven't found a good way to modify services and characteristics after they have been defined. The following line in the documentation makes me think there are some limitations to rearranging things after the initial setup: "It is currently only possible to add a descriptor to the last added characteristic (i.e. only sequential population is supported at this time)."
I'll post here if I find out otherwise.

Thanks - don't worry about the ack too much - it might be possible to do it in the same way as the write handler, but to be honest the callbacks could probably do with a bit of work.

MVP support for indication is available in #935.

I'll keep an eye out for ways to modify the attribute table but I don't have high hopes.
The current situation is problematic because calling setServices more than once with the same values seems to corrupt the attribute table and leads to "funky" behaviour. Maybe ignoring existing characteristics in setServices with a loop similar to the one implemented in updateServices would help?
That way, it would be possible to add new characteristics later on but not to modify or remove existing ones. The downside is it makes the semantics of setServices harder to grasp.

Thanks!

I just made a new bug for the setServices issue: #936

Something needs to be done, because if you do reset() you'd hope that everything will go back to normal - including the services. Right now the only way to undo it is a hard reset!

As you say, I doubt we can modify, but I'd hope that the attribute table could be wiped clean - even if that involved resetting the softdevice.

Fixed :)