Revert the change for reseting service data on non scan hci events

[lordthorzonus]

See the background discussion and logs what are happening here: #296

This commit b67eea2 changed the logic where the service data is reset from the meta event type scan to ever having a scan response discovered, and so leading to ever growing serviceData arrays in advertisements.

I reverted the logic to the one that was existing before this commit and introduced a test for catching the problem. I'm not so familiar with this codebase so this could use a another pair of eyes too 😅.

[Apollon77]

@kele6ra ... please have a look too. Your PR introduced the issue. Please advice.

[kele6ra]

Hello, I can't understand what you have changed. Because we have:

  if (type === LE_META_EVENT_TYPE_SCAN_RESPONSE) {
    hasScanResponse = true;
  }

And you haven't changed logic. Maybe in another file, but not in the gap.js.
It was very long ago, and I don't remember what I have done there ))
But in the 276 row you have leMetaEventType instead of type . It could be a reason.

[lordthorzonus]

@kele6ra 👋 the hasScanResponse is already set true above it:

noble/lib/hci-socket/gap.js

Lines 131 to 133 in 67e3936

	let hasScanResponse = previouslyDiscovered
	? this._discoveries[address].hasScanResponse
	: false;

So in case we receive an event 0x04 (scan) and then 0x01 the serviceData array is not reset cause because of hasScanResponse is then true. One can reproduce this with the test that I added, if we rely on the hasScanResponse the test shows that the serviceData array just keeps growing on new events 🤔

flowchart TD
 event1[Event 0x04] --> advertisement1[This peripheral is now previously discovered and its hasScanResponse is true]
 advertisement1 --> event2[Some other event received]
 event2--> nextStep[The service data should be reset, but it's not as hasScanResponse is true as this is discovered already]
 nextStep--> loop[This loops and service data just gets appended]

Loading

It looks like this in real life:

2023-02-21T19:06:01.383Z gap advertisement = {"localName":"Flower care","serviceData":[{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,23,156,6,110,141,124,196,13,7,16,3,115,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,23,156,6,110,141,124,196,13,7,16,3,115,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,23,156,6,110,141,124,196,13,7,16,3,115,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,23,156,6,110,141,124,196,13,7,16,3,115,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,23,156,6,110,141,124,196,13,7,16,3,115,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,23,156,6,110,141,124,196,13,7,16,3,115,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,23,156,6,110,141,124,196,13,7,16,3,115,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,23,156,6,110,141,124,196,13,7,16,3,115,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,23,156,6,110,141,124,196,13,7,16,3,115,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,24,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,24,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,24,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,24,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,24,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,24,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,24,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,25,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,25,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,25,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,25,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,25,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,25,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,25,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,25,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,25,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,26,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,26,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,26,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,26,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,26,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,26,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,26,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,27,156,6,110,141,124,196,13,7,16,3,134,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,27,156,6,110,141,124,196,13,7,16,3,134,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,27,156,6,110,141,124,196,13,7,16,3,134,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,27,156,6,110,141,124,196,13,7,16,3,134,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,28,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,28,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,28,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,28,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,28,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,28,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,28,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,28,156,6,110,141,124,196,13,8,16,1,35]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,29,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,29,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,29,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,29,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,29,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,29,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,29,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,29,156,6,110,141,124,196,13,9,16,2,197,1]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,30,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,30,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,30,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,30,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,30,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,30,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,30,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,30,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,30,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,30,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,30,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,30,156,6,110,141,124,196,13,4,16,2,234,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,31,156,6,110,141,124,196,13,7,16,3,97,0,0]}},{"uuid":"fe95","data":{"type":"Buffer","data":[113,32,152,0,31,156,6,110,141,124,196,13,7,16,3,97,0,0]}}],"serviceUuids":["fe95"],"solicitationServiceUuids":[],"serviceSolicitationUuids":[]}

But reverting this logic to rely just on the event seems to fix this

[kele6ra]

Sorry, but I can't check it now.
Looks like you are right.

[Apollon77]

I will also look into the PR later today hopefully ... but seems legit to me - and yes the ever growing servicedata is a bad bug right noe ... @rzr release work incoming ;-))

[Apollon77]

LGTM

@rzr