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

Add Read Numlock/Capslock/Scrolllock state #70

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

Conversation

Cemu0
Copy link

@Cemu0 Cemu0 commented Jan 26, 2021

Thank you for your repository, I transformed my wire keyboard to wireless :)
When type on the numpad the Numlock stage should be sync or the numpad just run wrong, same with caplock stage.
I also added an example for demonstration.
Hope this helpful

@T-vK
Copy link
Owner

T-vK commented Jan 26, 2021

ESP32_USBHOST_TO_BLE_KEYBOARD looks amazing. I love it!
I hope I can find some time to review the PR it this week. What kind of devices have you tested this with? Does it work with Windows, Android, Linux, iOS, Mac OS? I try to make sure that it at least works with Windows, Linux and Android before accepting a PR.
I can only do testing for Linux and Android, as I don't have a Windows machine.

@Cemu0
Copy link
Author

Cemu0 commented Jan 26, 2021

I tested and this PR work well with my Windows PC (ThinkPad P51) and iPhone6s (ios 14.3). But with the iphone, "sometime" the ESP32 not appear on the pair list on Iphone so I have to use a external App like (BLE Scanner) to pair the phone with ESP32). Currently I don't have a Linux or Android for test.

@lalalandrus
Copy link

lalalandrus commented Jan 27, 2021

I tried the code (downloaded the PR and updated the INO), which is way beyond my skillset, but I have issues.

  1. I am getting panics in various places whenever the event handler gets called
    [D][BLEDevice.cpp:102] gattServerEventHandler(): gattServerEventHandler [esp_gatt_if: 4] ... Unknown
    [V][BLEUtils.cpp:153Guru Meditation Error: Core 1 panic'ed (StoreProhibited). Exception was unhandled.

or

[FreeRTOS.cpp:63] wait(): >> wait: Semaphore waiting: name: CreateEvt (0x3ffd7a4c), owner: executeCreate for executeCreate
[D][BLEDevice.cpp:102] gattServerEventHandler(): gattServerEventHGuru Meditation Error: Core 1 panic'ed (StoreProhibited). Exception was unhandled.

  1. i noticed there is a mandatory delay of 1s in the setup. my application requires a really fast reconnect and was wondering if there is any way to pare that down. without addressing my panics mentioned above, i am unable to test.

Update: Looks like the delay in the startup on my ESP requires 1.5s. Any suggestions on how to optimize and reduce this delay is appreciated.

@LeeNX
Copy link

LeeNX commented Jan 27, 2021

ReadWriteNumLock.ino needs to go into it's own folder named ReadWriteNumLock, that is how Arduino IDE works.

Might be worth adding a little more info too ReadWriteNumLock.ino. I also think rather using Global variables in KbdLedCb as I did for testing

uint8_t CapsLockState = 0;

//note that this function should run "fast" or esp will crash
void KbdLedCb(KbdLeds *kbls)
{
    //digitalWrite(2,kbls->bmNumLock);
    // digitalWrite(2,kbls->bmCapsLock);
    // digitalWrite(2,kbls->bmScrollLock);
    // ...
    CapsLockState = kbls->bmCapsLock;
}

I updated my loop test as follows

void loop() {
  if(bleKeyboard.isConnected()) {
    Serial.println("CAPS LOCK State");
    Serial.println(CapsLockState);
    Serial.println("Writting CAPS LOCK");
    bleKeyboard.write(KEY_CAPS_LOCK);
    Serial.println(CapsLockState);
    delay(1000);
  }

  Serial.println("Waiting 5 seconds...");
  delay(5000);
}

Which shows me when the CapsLock has been changed, but the bleKeyboard.write(KEY_CAPS_LOCK); always turns it off, as a newbie, I am not sure how to toggle the CapsLock, thou I am more interested in reading the CapsLock.

I have a few ideas on how to monitor the Global variable for change, this might not be the correct place to discuss that.

I can confirm the ESP32 to macOS Catalina (v10.15.7) MacBook Pro (15-inch,2017) works with this PR.

Thanks @Cemu0 and @T-vK

@lalalandrus
Copy link

I know the folder structure.

I have narrowed the issue down to the setup delay. This delay also causes the issue where if the debug disabled, the initial interrogation of the states happens faster than the callback is able to be set allowing the data to be lost. Setting the callback prior to bleKeyboard.begin() does not work.

@lalalandrus
Copy link

lalalandrus commented Jan 27, 2021

Ok i modified the library where the blekeyboard has a private declaration of the callback (as defined in keyboardoutputcallbacks). Thus the callback now must be set prior to begin:

void BleKeyboard::setLedChangeCallBack(void (*func)(KbdLeds*))
{
  this->func = func;
}

or invoke an overloaded begin function:

void BleKeyboard::begin(void (*func)(KbdLeds*))
{
  this->func = func;
  xTaskCreate(this->taskServer, "server", 20000, (void *)this, 5, NULL);
}

Then right when the code allocates the mem for:
bleKeyboardInstance->keyboardOutputCallBack = new KeyboardOutputCallbacks();

we immediately assign the callback
bleKeyboardInstance->keyboardOutputCallBack->func = bleKeyboardInstance->func;

If there is a better way, please let me know. Thanks everyone for the help and dev.

@Cemu0
Copy link
Author

Cemu0 commented Jan 27, 2021

@lalalandrus

Update: Looks like the delay in the startup on my ESP requires 1.5s. Any suggestions on how to optimize and reduce this delay is appreciated.

I considered the connection time in my code here which can lower the delay time after BleKeyboard::begin() down about 100ms (or even lower, i didn't tested) Sorry if that code like a messy since I tried to force the Esp connecting with multiple device but fail.
I don't want to change the original code for backward compatible and I not sure that all device could work if I change the taskServer in that way.
For fast connection I think the delay after begin wouldn't affected the speed of connection since all the BLE was handled by other underground task.
@LeeNX
On Windown 10, the bleKeyboard.write(KEY_CAPS_LOCK); should toggle the CapsLock key and usually, with that example code it call back the KbdLedCb instantly, I not sure but i guess it could related to macOS ? could you please try bleKeyboard.press(KEY_CAPS_LOCK); or bleKeyboard.release(KEY_CAPS_LOCK);.

@lalalandrus
Copy link

I agree the delay would not impact connection, but it does prevent the main loop from running. The proposed change would reduce the dependency and any chance of kernel panics.

I will have a look at your code, will take some time to digest.

@LeeNX
Copy link

LeeNX commented Jan 31, 2021

Just as a note,

  if(bleKeyboard.isConnected()) {
    Serial.println("CAPS LOCK State");
    Serial.println(CapsLockState);
    Serial.println("Writting CAPS LOCK");
    //bleKeyboard.write(KEY_CAPS_LOCK);
    bleKeyboard.press(KEY_CAPS_LOCK);
    Serial.println(CapsLockState);
    delay(1000);
    bleKeyboard.release(KEY_CAPS_LOCK);
    //bleKeyboard.releaseAll();
    Serial.println(CapsLockState);
    delay(1000);
    Serial.println(CapsLockState);
  }

Thou it's not directly related to the LED states PR, but info. OSX does not toggle the CapsLock state with bleKeyboard.write(KEY_CAPS_LOCK);, I had to do the press and release thing as shown above.

As for the LED states PR, looks like it works perfectly with Android, thou it was not so easy to test. I could not find a on-screen keyboard that would reflect the CapsLock state, thou when writing the state, that did update on the Android device. I had to connect an external USB keyboard, that had CapsLock LED and I could see the CapsLock LED match the state in the ESP32 device when toggled with the USB keyboard.

@fangfufu
Copy link

fangfufu commented Jun 26, 2021

@T-vK , what's the status of this pull request? Are there any outstanding issues that stop you from merging it? 🙂

@T-vK
Copy link
Owner

T-vK commented Jun 27, 2021

@fangfufu Yes, I'd like to get this one sorted out first: arduino-libraries/Keyboard#43

@mcer12
Copy link

mcer12 commented Jan 3, 2022

@T-vK Sorry to jump into the conversation like this but I see this hasn't been merged yet. Since this is - at least to me - critical feature, I think it would make sense to merge this rather than keep waiting for the other library author, it's been months already :)

@60999
Copy link

60999 commented May 8, 2022

Excuse me, what's the progress now? Has the problem been solved?

@T-vK
Copy link
Owner

T-vK commented May 17, 2022

No, I'm still waiting for arduino-libraries/Keyboard#43 to be resolved. Kinda sad that it's taking so long. :/

@LeeNX
Copy link

LeeNX commented May 17, 2022

Just keeping an eye on this. Thanks @T-vK

@gimdh
Copy link

gimdh commented Jul 17, 2022

Thank you for the contribution. However, it seems the original Arduino Keyboard library is not going to be patched accordingly in near future. It would be great to at least maintain another branch with lock state support since it is such an important feature to build a complete keyboard yet delayed over a year.

@kilr00y
Copy link

kilr00y commented Aug 3, 2022

Could somebody point me in the right direction on how to fumble this feature into the current master branch?
I would be needing this feature for an ESP-Keyboard project but lack the c++ Skills to understand exactly what's going on.
I've been trying for a week now to no avail. Any pointers in the right direction would be appreciated.

@mcer12
Copy link

mcer12 commented Aug 3, 2022

@kilr00y First step to success is to learn to use github ;) You should be able to just use @Cemu0 's repository. https://github.com/Cemu0/ESP32-BLE-Keyboard

@kilr00y
Copy link

kilr00y commented Aug 3, 2022

@mcer12 been there, done that. Cemu0's repository is 2 years old and i have issues that are fixed in current.
The old commit is incompatible with the current master.

@kilr00y
Copy link

kilr00y commented Aug 4, 2022

nevermind... sometime you just have to ask to reach the solution yourself :)

@Cemu0
Copy link
Author

Cemu0 commented Sep 8, 2022

@T-vK it looks like adding keyboard-led call back won't be solved in near future, so as @kilr00y pointed out, I think the callback feature is quite overkilling for this simple task since most of we just need to indicator the CAPLOCK_LED or so.
As saw arduino-libraries/Keyboard/pull/61 (should be merged in near future)I think it is better to just give the user a way to check the led stage. Referent to this fork I came up with this brand . It can be used like this:

  if(bleKeyboard.isConnected()) {
    if (bleKeyboard.getLedStatus(LED_CAPS_LOCK))
    {
      Serial.print("Cap Lock on ");
    }
    if (bleKeyboard.getLedStatus(LED_SCROLL_LOCK))
    {
      Serial.print("Scroll Lock on "); //not work
    }
      if (bleKeyboard.getLedStatus(LED_NUM_LOCK))
    {
      Serial.print("Num Lock on ");
    }
  }

The BleKeyboard::onWrite does not call back when ScrLk changes on PC. So I think this is because NimBLE or PC does not support this key since I remember the last version it still works. So it would be helpful if test on other devices.
And if this is ok I will merge this into the master brand. Thanks.

Update: Sorry it's because of my cheap keyboard, the ScrLk button is fake and does not work in excel, I replaced it with a Logitech keyboard and everything works well. Even using bleKeyboard.write(KEY_SCROLL_LOCK).

@codemaster010
Copy link

i made a combination of others and your code, i was wondering if i should post it and then link here, sorry, im kinda new and im not familiar. i think it could be of help to some folks and it took me a long time to figure it all out. its not perfect but i think with yalls help a lot could be ironed out but its working pretty smoothly. sorry if this is the wrong place but it does have to do with the topic at hand. im just not familiar with the licensing or anything like that.

@codemaster010
Copy link

I think some.of above issues you guys mentioned might be solved with the code I have made like with the cap locks @T-vK @Cemu0

@LeeNX
Copy link

LeeNX commented Sep 12, 2024

@codemaster010 did you fork the repo and make changes we can look at?

@LeeNX
Copy link

LeeNX commented Sep 12, 2024

As a side note - Could also look at ESP32-BLE-CompositeHID, which I think has all this merged in, with mouse and gamepad too.

@codemaster010
Copy link

codemaster010 commented Sep 14, 2024

@LeeNX its not an exact copy but is it best to do fork or start another? im new to github uploading, is there any specific things i need to do in the proccess? it also uses another code thats has open license. i think as long as i give credit to original people should be ok right? my apologies for the newbie questions, just trying to do it right. i think the fork is the way to go

@LeeNX
Copy link

LeeNX commented Sep 15, 2024

@codemaster010 no worries. My go to would be to make a fork, possible also make a branch and put your changes in that. Fork would be easier to pull in upstream changes and making Pull Requests back to up stream. Can also update your fork's default branch to point to updated ReadMe and working code, with notes on why it works.

In your PR you can point out how and why your changes are better or more tested that this PR.

GitHub and git can be quite a big task to become comfortable with, but well worth it in the long run. Hope to see more feedback from you.

@codemaster010
Copy link

codemaster010 commented Sep 16, 2024

@LeeNX i used fabgl for the keyboard and some other things, so the code is a bit different and im sure there are mistakes but i added code to compensate and seems to run pretty decent just not native like the buttons should be pushed. since the code i made with gpt has many different workings but based on the same and similar principles to get the job done. My project just made it work with ps/2 when i could not find it online to do it. i found @Cemu0 fork a few days ago and im blown away but still his does not use ps/2 that im aware so i made my own version. i also made it work with some really cool hardware with extra fun and usefull features that i havnt seen in most keyboards. if you think its ok to fork ill do that asap, thank you for all the advice. The topics of Pull requests all the github terms are are kind above my head but doing my best to understand. hopefully i can remove the fork if its not done correctly for some reason. . its likely this build uses less power due to not needing a hid interface. I hope to see some feedback to see what you think. i have two code, one is more of a test and was wondering if that can or should be added too or can i share that in pull requests in the future? update, here is the link to the branch. the issues and features are in the commit description. there are more things to mention but thats what i have for now. https://github.com/codemaster010/ESP32-BLE-Keyboard-PS2-/blob/PS2-FabGL-BLE-Keyboard/BLEKeyboardPS2.ino

@codemaster010
Copy link

here is an updated readme file that hopefully satisfies for the most part the requirements needed as mentioned above. all feedback is highly welcomed. https://github.com/codemaster010/ESP32-BLE-Keyboard-PS2-/blob/PS2-FabGL-BLE-Keyboard/BLEKeyboardPS2.txt

@codemaster010
Copy link

After reviewing the way things work i am still unclear if i should of made a independent repository with attribution and everything like that since the main part of the code works in different ways and combines other libraries, if anyone could help clarify that would be greatly appreciated.

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.

10 participants