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

Remove overwrite of an already discovered characteristic #383

Closed
wants to merge 2 commits into from

Conversation

tripish
Copy link

@tripish tripish commented Apr 15, 2016

When using libraries based on noble-device, we've found a freezing bug that occurs during connectAndSetup() calls.

The call stack was something like

Device.prototype.connectAndSetup
NobleDevice.prototype.connectAndSetup
(create "notify" listener with self.notifyCharacteristic)
NobleDevice.prototype.discoverServicesAndCharacteristics
Noble.Peripheral.discoverAllServicesAndCharacteristics
(some events triggering)
(overwriting characteristics within
NobleDevice.prototype.discoverServicesAndCharacteristics)
(now it gets stuck waiting for "notify")

Within NobleDevice.prototype.notifyCharacteristic
(noble-device/lib/noble-device.js) the device is getting a reference
to the characteristic with
this._characteristics[serviceUuid]characteristicUuid and
waiting for that characteristic's "notify" event to call the callback.

The problem being that if the connection fails, and it goes through a
rediscover/reconnect, inside
NobleDevice.prototype.discoverServicesAndCharacteristics we can see it
overwriting the references for those characteristics
(this._characteristics[serviceUuid][characteristic.uuid] =
characteristic). So now if things run smoothly, within
NobleDevice.prototype.notifyCharacteristic it does another lookup
(:117 again) and now has a brand new reference to a newly created
characteristic. It emits "notify" on that new characteristic. But the
original call to connectAndSetup is still waiting on the old
characteristic object.

@sandeepmistry
Copy link
Collaborator

@tripish is there any reason you need to call both discoverServicesAndCharacteristics and discoverAllServicesAndCharacteristics?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants