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

NimBLE-Arduino (optional) and remove of xTaskCreate #111

Merged
merged 6 commits into from
Sep 10, 2021
Merged

NimBLE-Arduino (optional) and remove of xTaskCreate #111

merged 6 commits into from
Sep 10, 2021

Conversation

sivar2311
Copy link
Collaborator

NimBLE-Mode

This implements NimBLE-Mode to reduce memory consumption
ArduinoIDE: Before including the library, insert the line #define USE_NIMBLE

#define USE_NIMBLE
#include <BleKeyboard.h>

PlatformIO: Change your platformio.ini to the following settings

lib_deps = 
  NimBLE-Arduino

build-flags = 
  -D USE_NIMBLE

Comparison (SendKeyStrokes.ino at run-time)

Standard NimBLE mode difference
ESP.getHeapSize() 296.804 321.252 + 24.448
ESP.getFreeHeap() 143.572 260.764 + 117.192
ESP.getSketchSize() 994.224 579.264 - 414.960

xTaskCreate

xTaskCreate was removed to fix stack issues.

removed xTaskCreate in begin()
removed BleConnectionStatus
removed KeyboardOutputCallbacks
implemented callbacks directly in BleKeyboard
@T-vK
Copy link
Owner

T-vK commented Aug 27, 2021

Awesome! Once it has been tested with Windows, Mac OS, Linux, Android and iOS I will accept it.
I can only do the testing for Android and Linux. Do you have access to other platforms?

@sivar2311
Copy link
Collaborator Author

I only have the option to test Windows and Android.
I have already tested it on Windows.

@T-vK
Copy link
Owner

T-vK commented Aug 27, 2021

Okay, I will ping some people from other issues to give it a try.
There is one more thing. A lot of people have connection issues because the default device name is longer than 15 characters.
I think it would be a good idea to finally fix that and also automatically trim the given device/manufacturer name down to the first 15 characters in the constructor.

If you don't mind, I would like to have that fix added to this PR as well. This way we know that if it doesn't work for someone, it isn't because of the device name.

This should do:

BleKeyboard.cpp

BleKeyboard::BleKeyboard(std::string deviceName, std::string deviceManufacturer, uint8_t batteryLevel) 
    : hid(0)
    , deviceName(deviceName)
    , deviceManufacturer(deviceManufacturer)
    , batteryLevel(batteryLevel) {

        if (deviceName.length() > 15) {
            this->deviceName = deviceName.substr(0,15);
        }
        if (deviceManufacturer.length() > 15) {
            this->deviceManufacturer = deviceManufacturer.substr(0,15);
        }
...

BleKeyboard.h

...
BleKeyboard(std::string deviceName = "ESP32 Keyboard", std::string deviceManufacturer = "Espressif", uint8_t batteryLevel = 100);
...

@sivar2311
Copy link
Collaborator Author

I also did a test on Android and again encountered timing problems with NimBLE (it is too fast for my Samsung Galaxy 71).
I had to increase the delay time in vTaskDelay.
It seems that this timing must be different for some devices and should be made more flexible by adding a function like setDelay (or maybe a better name).

What do you think about this and what kind of name would you use for this function?

@sivar2311
Copy link
Collaborator Author

About the deviceName and manufacturerName: implemented in 65b6563 and 8b40e0c

@T-vK
Copy link
Owner

T-vK commented Aug 27, 2021

I agree, we should have a function like that. I'm not sure what to call that function either. Maybe setKeyDelay. That's what Autohotkey calls their version of it.
But technically spekaing vTaskDelay is probably doing way more than setting the delay between key events, so I'm not sure.
Another thing to consider is if this should also be implemented on ESP32-BLE-Mouse and ESP32-BLE-Gamepad. If so, then it would be better to find a common name. (setKeyDelay doesn't really work for scrolling or mouse cursor movements.)

Edit:
I think in addition to adding that function, we should also make the the default high enough to make it work on "most" devices.

Edit:2
I'd be okay with setKeyDelay because we probably don't need it in ESP32-BLE-Mouse and ESP32-BLE-Gamepad.

@sivar2311
Copy link
Collaborator Author

But technically spekaing vTaskDelay is probably doing way more than setting the delay between key events, so I'm not sure.

I have used vTaskDelay only because the arduino delay is not available (Arduino.h is not included).
If you have a better idea, let me know :)

I think in addition to adding that function, we should also make the the default high enough to make it work on "most" devices.

I agree. We just have to find out how high this value has to be.
On my Windows system, a value of 1 was already enough.
This was not enough on my Samsung Galaxy (but I have not yet determined an exact value here).

I'd be okay with setKeyDelay

Perfect :)

@sivar2311
Copy link
Collaborator Author

I have changed my mind. setDelay is a more neutral name ;)
For this I implemented a simple delay_ms function.
At the moment this is only used in NimBLE mode.

My tests have shown the following settings for a stable transmission:
Windows 2 ms
Android 7 ms

For this reason I have taken a default value of 7 ms.

However, this may be different on different PCs / Android phones.

@T-vK
Copy link
Owner

T-vK commented Aug 28, 2021

Great, let's go with 7 as a default then. If this doesn't work for everyone, I'll find out eventually and increase it.

@sivar2311
Copy link
Collaborator Author

3 milliseconds on my son's iPhone 8.

@T-vK
Copy link
Owner

T-vK commented Aug 30, 2021

Can you add this line to the library.properties file?

depends=NimBLE-Arduino

Edit:
Btw, I tested it on Linux and Android and it works fine. Both with NimBLE and without. The only issue I noticed is that when I turn Bluetooth off on Linux without manually disconnecting first, the disconnect even never gets fired resulting in isConnected() to keep returning true. And making a new connecting from another device doesn't work either in that state. So I had to restart my ESP. I think this is not a new problem though.

@sivar2311
Copy link
Collaborator Author

Can you add this line to the library.properties file?
depends=NimBLE-Arduino

I am not sure if this is a good idea as NimBLE-Arduino is optional.
I also had the idea to create a library.json for PlatformIO and to add the NimBLE-Arduino in the dependencies section. But I haven't done it yet for the same reason.

The only issue I noticed is that when I turn Bluetooth off on Linux without manually disconnecting first, the disconnect even never gets fired resulting in isConnected() to keep returning true.

This looks like a Linux problem to me.
On Windows and Android the onDisconnect callback is triggered properly as soon as Bluetooth is turned off on the client side.

@T-vK
Copy link
Owner

T-vK commented Aug 30, 2021

I am not sure if this is a good idea as NimBLE-Arduino is optional.
I also had the idea to create a library.json for PlatformIO and to add the NimBLE-Arduino in the dependencies section. But I haven't done it yet for the same reason.

I agree, I'm not sure either. I guess we leave it as is for now. But it might make sense to make NimBLE the default at some point.

This looks like a Linux problem to me.
I don't know, I feel like Windows and Android are probably sending a disconnect signal to the ESP before turning off.

If the Windows machine crashed or lost power while being connected you'd probably end up in the same state. So while it would probably seemingly fix the problem if Linux were to send that disconnect signal, I feel like the ESP should be capable of recovering from an unannounced disconnect. But I don't think this issue has anything to do with your PR.

@sivar2311
Copy link
Collaborator Author

I don't know how Windows would behave in the event of a crash (the last time was several years ago).
But the original problem was switching off the Bluetooth - so it's a different situation.

I don't know how to determine whether a BLE connection still exists.
Then a polling or something similar would have to take place regularly.
That will hardly be possible with BLE, or it would be counterproductive with regard to the "low energy" concept.

The problem is probably that the client does not disconnect the connection cleanly. But you can't blame the host for that ;)

I don't know enough details about how BLE works on the ESP. I am only a user of the (Nim)BLE library.

@T-vK
Copy link
Owner

T-vK commented Aug 30, 2021

I will find a way to address that issue eventually, I mean other BLE devices manage to do it too, but it's a low priority at the moment. :)

@sivar2311
Copy link
Collaborator Author

Yes, I think so too.
One step at a time... :)

@T-vK T-vK merged commit f4e0112 into T-vK:master Sep 10, 2021
@tahunus
Copy link

tahunus commented Oct 24, 2021

Hi @sivar2311 . I built a BLE keyboard that uses capacitive sensors instead of keys (with Adafruit's MPR121 Sensor Breakout) and Adafruit's ESP32-WROOM-32 as MCU (HUZZAH32). These two communicate over I2C and had noise issues that were fixed on Espressif's Arduino-ESP32 Rel 2.0.0. (that's based on the latest development version of ESP-IDF to attain Thread-Safe I2C, here: https://github.com/espressif/arduino-esp32/releases/tag/2.0.0 ). I'm using T-vK's ESP32-BLE-Keyboard v0.3.0 and everything runs smoothly. When trying to use nimBLE (to save space and add other functions), I got compilation errors from nimBLE files not found. Because the Arduino-ESP32 is based on IDF, I copied the nimBLE files from here https://github.com/h2zero/esp-nimble-cpp but didn't work. h2zero states to install ESP-IDF v4.0+ but I think that could mess up Espressif's Arduino-ESP32 Rel 2.0.0? What combination of Arduino-ESP32 implementation & nimBLE files should I use to run BLE-Keyboard on nimBLE? Many thanks! (just FYI: this is an attempt to create a wireless "RPM Letterboard", something used in some autistic therapies)

@sivar2311
Copy link
Collaborator Author

Hi @tahunus

I think you have installed the wrong library.
You need to install NimBLE-Arduino
The repository you mentioned, is not made for Arduino.

If you are on ArduinoIDE, you can simply install the library via the library manager.

@tahunus
Copy link

tahunus commented Oct 29, 2021

Hi @sivar2311
I was indeed using the wrong library. I installed NimBLE-Arduino and no compilation errors. Thanks!

Follow-up question: I don't see any savings in memory usage.

This is the compiling result with regular BLE:
Sketch uses 968897 bytes (73%) of program storage space. Maximum is 1310720 bytes.
Global variables use 22860 bytes (6%) of dynamic memory, leaving 304820 bytes for local variables. Maximum is 327680 bytes.

This is the compiling result when adding the line #define USE_NIMBLE before the #include <BleKeyboard.h> statement:
Sketch uses 979853 bytes (74%) of program storage space. Maximum is 1310720 bytes.
Global variables use 23212 bytes (7%) of dynamic memory, leaving 304468 bytes for local variables. Maximum is 327680 bytes.

Below is the full sketch for your reference.
What am I missing?
Thanks again.

#include <Wire.h>
#include <Adafruit_MPR121.h>
#ifndef _BV
#define _BV(bit) (1 << (bit))  
#endif
Adafruit_MPR121 cap1 = Adafruit_MPR121();
Adafruit_MPR121 cap2 = Adafruit_MPR121();
Adafruit_MPR121 cap3 = Adafruit_MPR121();

#define USE_NIMBLE
#include <BleKeyboard.h>
BleKeyboard bleKeyboard("RPM Letterboard V1.0");

bool keybON = false;
uint16_t lastT1, currT1 = 0;
uint16_t lastT2, currT2 = 0;
uint16_t lastT3, currT3 = 0;
int capT, senT = 0; //capT=Each mpr121(0 to 2), senT=Sensors(0 to 11)
char L[12][3]; //adding one for null char in char array
bool action = false;
float battRead;

void loadLetterTable() {
  for (int i=0;i<3;i++){
    for (int j=0;j<12;j++){
      L[j][i] = ' '; }}
  L[4][2]='a'; L[2][1]='b'; L[3][1]='c';  L[8][1]='d'; L[0][0]='e';
  L[5][2]='f'; L[1][1]='g'; L[4][1]='h';  L[9][1]='i'; L[1][0]='j';
  L[3][2]='k'; L[8][2]='l'; L[5][1]='m'; L[10][1]='n'; L[3][0]=';';
  L[2][2]='o'; L[7][2]='p'; L[6][1]='q'; L[11][1]='r'; L[4][0]='s';  L[9][0]='t';
  L[1][2]='u'; L[6][2]='v'; L[7][1]='w';  L[2][0]='x'; L[5][0]='y'; L[10][0]='z';
  L[0][2]= 8;                                                       L[11][0]= 32;     //BS & Spacebar
}

void Buzz(int tono, int timelapse, bool full) {
  ledcWriteTone(0,tono);
  delay(timelapse);
  if (full) {ledcWrite(0,0);}
}

void setup() {
//  Serial.begin(115200);
  loadLetterTable();   
  pinMode(25,OUTPUT);       //ledc for Buzzer : Arduino's TONE not implemented for ESP32
  ledcSetup(0,100000,12);   //Parms=(channel, freq, resolution)
  ledcAttachPin(25,0);
  bleKeyboard.begin();
  Wire.begin (23,22,100000);                                                          //clock speed defualt =100k, min=5k, max=400k
                                                                                      //Default 0x5A, 3.3V->0x5B, SDA->0x5C, SCL->0x5D
  if (!cap1.begin(0x5A)) {Buzz(100,300,true); while (1); }                            //Serial.println("5A Bad");
   else {Buzz(1000,100,true);}                                                        //Serial.println("5A Good"); }
  if (!cap2.begin(0x5D)) {Buzz(100,300,true); while (1); }                            //Serial.println("5D Bad"); 
   else {Buzz(1500,100,true);}                                                        //Serial.println("5D Good"); }  
  if (!cap3.begin(0x5C)) {Buzz(100,300,true); while (1); }                            //Serial.println("5C Bad"); 
   else {Buzz(2000,100,true);}                                                        //Serial.println("5C Good"); }  
//cap1.setThresholds(0x0C,0x06);                                                      //touch=20, release=8. Default t=12, r=6
//cap2.setThresholds(0x14,0x08);
//cap2.setThresholds(0x14,0x08);
}

void loop() {
  while (!bleKeyboard.isConnected()) {
    Buzz(200,30,false); Buzz(50,60,true);
    keybON=false; 
    delay(2000);}
  if (bleKeyboard.isConnected() && !keybON) {
    Buzz(800,200,false); Buzz(1000,300,true);
    keybON=true;}
  battRead=analogRead(A13); // or analogRead(35)
  battRead=((battRead * 2) /4098) * 3.3;
  currT1 = cap1.touched(); currT2 = cap2.touched(); currT3 = cap3.touched();          //Get the currently touched pads
  for (int i=0; i<12; i++) { 
    if (cap1.filteredData(i)>1000) {Buzz(100,300,true);}                              //error    
    if (cap2.filteredData(i)>1000) {Buzz(100,300,true);}                              //error    
    if (cap3.filteredData(i)>1000) {Buzz(100,300,true);}                              //error                            
    if ((currT1 & _BV(i)) && !(lastT1 & _BV(i)) ) {capT=0;senT=i;action=true;}        //if it *is* touched--   
    else if ((currT2 & _BV(i)) && !(lastT2 & _BV(i))) {capT=1;senT=i;action=true;}    //and *wasnt* touched before--
      else if ((currT3 & _BV(i)) && !(lastT3 & _BV(i))) {capT=2;senT=i;action=true;}  //then, it is touched now   
  }
  lastT1 = currT1;                                                                    //reset cap1 state  
  lastT2 = currT2;                                                                    //reset cap2 state  
  lastT3 = currT3;                                                                    //reset cap3 state
  //delay(40);                                                                        //in case repeated words too fast
  if (action) {
      bleKeyboard.print(L[senT][capT]);}
  action=false;
}

@sivar2311
Copy link
Collaborator Author

It looks like the #define statement in the ArduinoIDE is not working :(
I am using PlatformIO. There you can set a corresponding define in the platformio.ini file.
I'm trying to find out if there is any other way to solve this in the ArduinoIDE.

@sivar2311
Copy link
Collaborator Author

Workaround for Arduino IDE:
Edit BleKeyboard.h and put #define USE_NIMBLE before the first line
Edit BleKeyboard.cpp and move line 17 before the first line

With this modifications the result from compiling the example is:

Sketch uses 553289 bytes (42%) of program storage space. Maximum is 1310720 bytes.
Global variables use 25516 bytes (7%) of dynamic memory, leaving 302164 bytes for local variables. Maximum is 327680 bytes.

@tahunus
Copy link

tahunus commented Oct 29, 2021

Thanks! Works as you mentioned!

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.

3 participants