Skip to content

replaced original BLE_iBeacon.ino example with a known-good one #7470

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
Merged
merged 4 commits into from
Dec 21, 2022
Merged

replaced original BLE_iBeacon.ino example with a known-good one #7470

merged 4 commits into from
Dec 21, 2022

Conversation

@Human
Copy link
Contributor

@Human Human commented Nov 13, 2022

Description of Change

I had no luck getting the original BLE_iBeacon.ino code to work as expected with iBeacon scanners, but the included reference code worked completely. Thanks to @31337Ghost for writing it! I am simply submitting the PR.

Tests scenarios

Tested on DOIT ESP32 DEVKIT V1, using various scanner client apps on Android devices.

Related links

Closes #7360
@CLAassistant
Copy link

CLAassistant commented Nov 13, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.
@VojtechBartoska VojtechBartoska added the Type: Example Issue is related to specific example. label Nov 14, 2022
@VojtechBartoska VojtechBartoska added this to the 2.0.6 milestone Nov 16, 2022
@P-R-O-C-H-Y
Copy link
Member

P-R-O-C-H-Y commented Dec 5, 2022

Hi @Human, I am testing the example and can say, that this new one works better.
But I have a question, after disconnecting ESP from phones APP the ESP disappears and need to hw reset. Do you have same behaviour and is that correct?

Tested on ESP32 + ESP32-C3
@P-R-O-C-H-Y
Copy link
Member

P-R-O-C-H-Y commented Dec 13, 2022

@Human Can you please take a look on my comment above? Thanks
@Human
Copy link
Contributor Author

Human commented Dec 13, 2022

@P-R-O-C-H-Y It will take me some time to be able to test this, but which app on which platform did you use?
VojtechBartoska
VojtechBartoska reviewed Dec 14, 2022
View reviewed changes
libraries/BLE/examples/BLE_iBeacon/BLE_iBeacon.ino
void loop() {
}
/*
Based on 31337Ghost's reference code from https://github.com/nkolban/esp32-snippets/issues/385#issuecomment-362535434
Copy link
Contributor

@VojtechBartoska VojtechBartoska Dec 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@Human Please, it seems a lot of lines are still similar to the example created by Neil Kolban.

Can you please mentioned in the header that this example is "modified version by 31337Ghost User " of the original example from Neil?

Thanks
Copy link
Contributor Author

@Human Human Dec 17, 2022

Choose a reason for hiding this comment

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

Please check revised comment.
@P-R-O-C-H-Y
Copy link
Member

P-R-O-C-H-Y commented Dec 14, 2022

@P-R-O-C-H-Y It will take me some time to be able to test this, but which app on which platform did you use?

@Human I did test on ESP32-C3 and with HONOR phone running Android 10 + app called nRF Connect (also tried another app, but got same behaviour).
@Human
Copy link
Contributor Author

Human commented Dec 17, 2022 •
edited
Loading

@P-R-O-C-H-Y I tried your recommended app, and I do not get a CONNECT option for the ESP32 iBeacon running this code. I do see the option for a BlueCharm beacon and my TV (for some reason). (See update below.)
@Human
Copy link
Contributor Author

Human commented Dec 17, 2022

@P-R-O-C-H-Y I was able to reproduce what you saw, but I don't know enough about BLE or iBeacons to fix the issue.
P-R-O-C-H-Y
P-R-O-C-H-Y requested changes Dec 19, 2022
View reviewed changes
Copy link
Member

@P-R-O-C-H-Y P-R-O-C-H-Y left a comment

Choose a reason for hiding this comment

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

@Human please take a look on my comment :)
@me-no-dev
Copy link
Member

me-no-dev commented Dec 19, 2022

@Human could you please rebase this PR git rebase -i master because it seems to include some commits that are already on master and is causing CLA to think that there are many contributors.
@Human Human force-pushed the replace_BLE_iBeacon branch from 3445c16 to 40bd410 Compare December 21, 2022 14:46
@P-R-O-C-H-Y P-R-O-C-H-Y merged commit e9c125f into espressif:master Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Example Issue is related to specific example.

5 participants

@Human @CLAassistant @P-R-O-C-H-Y @me-no-dev @VojtechBartoska