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

enable static pin as parameter for begin() #26

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

SvOlli
Copy link

@SvOlli SvOlli commented May 22, 2020

To have a simple way of protected pairing, an optional parameter can be
specified to used as a pin for authentication. Tested on a Linux system.

Signed-off-by: Sven Oliver Moll [email protected]

To have a simple way of protected pairing, an optional parameter can be
specified to used as a pin for authentication. Tested on a Linux system.

Signed-off-by: Sven Oliver Moll <[email protected]>
SvOlli pushed a commit to SvOlli/VICEboard that referenced this pull request May 22, 2020
- added support for PIN during pairing, needs change in library
  (pull request: T-vK/ESP32-BLE-Keyboard#26)
- refactoring of the menu code to support different number types
  (uint8_t, uint16_t, uint32_t) in decimal and hex
- a couple of new configuration parameters
- renaming of types (e.g. unsigned short -> uint16_t)
- custom keyboard mapping, if you want to use a different emulator as VICE
@T-vK
Copy link
Owner

T-vK commented May 25, 2020

Thanks for the pull request. I'm not sure if having the pin be the first parameter of the begin method is the best way of doing this. It could be though, I haven't really looked into it, but have you considered making the pin a constructor parameter, or maybe have a dedicated setPin method or simply a property and if so, what conclusion did you come to?
I'm a bit hesitant because once we add this, there is no way to maybe have something more important as the first parameter of the begin method in the future without breaking backwards compatibility.

@SvOlli
Copy link
Author

SvOlli commented May 26, 2020

The idea of using begin() was to have a similar to the begin() of the Serial class. Using the constructor is not a good idea, because the object is usually created before running main(), but you might want to read the PIN from EEPROM (simulated by flash). It is also necessary that the PIN is set before running the Bluetooth task, or the parameter is not used.
So the best thing that could be done is to create a setPIN() function that prints out an error if the Bluetooth task is already running, but that's not as elegant as the optional parameter for begin(). Since I only had an insight on my small change, but you have a much better overview of the whole library. So I'll implement whatever you think is best.
How about a separate beginPIN() method that will set the PIN and calls begin() afterwards?

@T-vK T-vK self-assigned this Jul 7, 2020
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.

2 participants