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.

Sign up for GitHub

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

Jump to bottom

More than 16 syncgroups cannot be advertised via BLE #1404

Open
azinman opened this issue Jul 20, 2016 · 5 comments
Open

More than 16 syncgroups cannot be advertised via BLE #1404

azinman opened this issue Jul 20, 2016 · 5 comments
Milestone
Syncbase HLAPI Co...

Comments

@azinman
Copy link

azinman commented Jul 20, 2016

The current BLE implementation has a hard limit on 16 advertisements. As each syncgroup is advertised on its own, it means we can only have 16 syncgroups advertised. This is a problem for many apps such as todos as each todo list has its own syncgroup.

@asadovsky @asimshankar @ultrasaurus @afandria
@azinman
Copy link
Author

azinman commented Jul 21, 2016 •
edited

Pending CL https://vanadium-review.googlesource.com/#/c/23864 disables BLE to avoid this problem.

@azinman
Copy link
Author

azinman commented Jul 21, 2016

Updated CL to ignore failure as long as 1 plugin succeeds -- BLE is re-enabled.

@ultrasaurus
Copy link

@afandria I would expect we would see the same issue on Android as well? or is the implementation substantially different?

vanadium-bot pushed a commit to vanadium-archive/go.ref that referenced this issue Jul 21, 2016
@azinman
ref: Prevent crashes when advertising >16 syncgroups
5e6777f 
Currently two related crashes will occur should the user have more
than 16 syncgroups. The root of the cause is Serve() will start
advertising each syncgroup, and if the platform has BLE then
some will fail as the current BLE implementation can only support
16 total advertisements. If BLE advertisement fails, one crash was
a null dereference of the advertisement's stop().  Note CL #23804
fixed this type of crash in one place in the function there was
another where stop could be nil and dereferenced.

The other crash is a vlog.Fatal should advertising syncgroups fail. This
CL allows advertising to continue (only logging errors) should at least
one plugin work. That way BLE can work up to its max, but mDNS can
continue advertising all syncgroups.  This is one solve the >16 BLE
advertisements issue with BLE, filed as vanadium/issues#1404.

Closes vanadium/issues#1390, vanadium/issues#1403, vanadium/issues#1398

Change-Id: I07fee516a5246ad8ff72e1107158c16f33d68562
@asimshankar
Copy link

The limit of 16 comes from:
https://github.com/vanadium/go.ref/blob/60698e61821e1905605ff2cabf4c230d33989386/lib/discovery/plugins/ble/encoding.go#L36

It is somewhat arbitrary, but the general idea was that we need to limit the total amount of data available from the GATT server. IIRC, @jhahn21 noted that the actual limit varies depending on the hardware and the software stack and 16 was an unscientifically chosen arbitrary bound by us.

Long story short though: Yes, this issue affects all platforms, it is not Android or iOS specific.

@azinman
Copy link
Author

azinman commented Jul 21, 2016

Would it be possible implement something like a bloom filter for the syncgroup ids or advertisement data, instead of listing all the data? Or at least in the case for >16 advertisements?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Syncbase HLAPI Complete
Development

No branches or pull requests
4 participants
@asimshankar @ultrasaurus @azinman @afandria