search

dbinfrago / libpax

Apache License 2.0
21 stars 13 forks source link

1.0.0 RC2: Feature add ons, bugfixes, use low level VHCI control instead of Bluedroid or Nimble stack #12

Closed cyberman54 closed 2 years ago

cyberman54 commented 3 years ago

@FlorianLudwig I briefly checked scanning results with the lib in this VHCI direct command control version, against the former Bluedroid stack controlled version. Results are slightly better. Thus, i suggest to merge this PR now for v1.0.0.

cyberman54 commented 3 years ago

@FlorianLudwig another update: i tested impact of BLE advertising, and did not see a significance for scanning results. Thus, i made BLE advertising optional by introducing a compiler directive. Default BLE advertising is now off.

FlorianLudwig commented 2 years ago

@cyberman54

I don't understand the purpose of the advertisement. Is the hypothesis that other devices do more advertisements when they also see advertisements?

cyberman54 commented 2 years ago

@FlorianLudwig Indeed that was the hypothesis. But i couldn't validate it, neither with active nor passive scanning, scan results did not differ significantly in my tests. That's why i made advertising optional in the code by adding a compiler directive.

Another use case for the advertising code, besides pax counting, could be to use a paxcounter as simple unconnectable BLE beacon in parallel, broadcasting some kind of data. That's why i kept the advertising code.

Lesson learned was, that there is no need of a full blown bluetooth stack, while we only want to scan and maybe advertise, without being connectable. Thus, the scan & advertise task can better be carried out on a lower level by the ESP32, so we only need some vHCI commands to enable this.

FlorianLudwig commented 2 years ago

@cyberman54 While it is an interesting experiment I would prefer to keep this code out of libpax as otherwise users of libpax might expect we maintain a low-level library for basic BLE functions. Sending advertisements is out of scope for libpax in my opinion.

So I would like to remove all unused functions from this MR before merging.

/cc @oliverbrandmueller

cyberman54 commented 2 years ago

ok, no problem. I can keep this in a separate branch in my fork repo, so nothing gets lost.

FlorianLudwig commented 2 years ago

@cyberman54 should I remove the code or will you update the MR?

cyberman54 commented 2 years ago

I would prefer you do it, then i can leave my fork untouched to keep the code there. Thank you.

cyberman54 commented 2 years ago

@FlorianLudwig i removed the advertising code from my branch HCI and this MR. Kept it in my new branch hci-advertise.

oliverbrandmueller commented 2 years ago

Code merged to master; 1.0 branch ahead

torfo commented 2 years ago

Hi @oliverbrandmueller Could you create a tag for the current version "1.0.0 RC2"?

oliverbrandmueller commented 2 years ago

@torfo We have a final fix pending (@FlorianLudwig is checking the default handling of counter mode) before releasing 1.0.0 which I would tag then.

cyberman54 commented 2 years ago

@oliverbrandmueller is the final fix eta known?

FlorianLudwig commented 2 years ago

@cyberman54 There has been a report of crashes with the current master and a 1.0.0 release has to wait until that is cleared.

I am missing on details on the issue though so i have no eta.

cyberman54 commented 2 years ago

can i get the report? I will take a look into, then.