Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extmod/modbluetooth: Enable configuration of rx buffer. #5268

Closed
wants to merge 3 commits into from

Conversation

jimmo
Copy link
Member

@jimmo jimmo commented Oct 28, 2019

Enables a straightforward way to use the NUS without worrying about losing writes if there's a delay calling gatts_read before the next write appears.

Thanks to @mirko for the suggestion.

return MP_EINVAL;
}
gatts_db_entry_t *entry = MP_OBJ_TO_PTR(elem->value);
entry->data = m_new(uint8_t, len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to use m_renew(uint8_t, entry->data, entry->data_alloc, len), to reuseresize memory where possible

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered using m_renew (and truncating data_len to the new data_alloc) to preserve existing data, but scrapped that because the intention is that it's only called once at registration.

But just for the purpose of possible reusing the allocation, makes sense. Done.

@dpgeorge
Copy link
Member

Great, it's very clean!

Just to play devil's advocate: why not put this buffer configuration in the services/chars structure passed to gatts_register_services?

- Adds an explicit way to set the size of a value's internal buffer,
  replacing `ble.gatts_write(handle, bytes(size))` (although that
  still works).
- Add an "append" mode for values, which means that remote writes
  will append to the buffer.
@jimmo
Copy link
Member Author

jimmo commented Oct 29, 2019

Just to play devil's advocate: why not put this buffer configuration in the services/chars structure passed to gatts_register_services?

Because that structure is complicated and ordinal-based, I'm very hesitant to add additional fields to it that only apply to a very small number of chars/descs. i.e. if we later add another field, then now everything has to provide defaults for the buffer params. (A named tuple or dict would address that, but much more complexity).

@dpgeorge
Copy link
Member

Thanks for the argument, I agree!

Merged in d16a27d through 25946d1

@dpgeorge dpgeorge closed this Oct 29, 2019
tannewt added a commit to tannewt/circuitpython that referenced this pull request Sep 1, 2021
Update tinyusb for USB Compliance Verification test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants