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

ESP32 garbage values #21

Closed
dharmik768 opened this issue Jun 20, 2021 · 9 comments
Closed

ESP32 garbage values #21

dharmik768 opened this issue Jun 20, 2021 · 9 comments

Comments

@dharmik768
Copy link

I am using ESP32 with this library to read my PS2 keyboard. I am using a bi-directional logic level shifter based on MOSFET to convert 3.3v to 5v logic shift.
The GPIO pin that I was using for the clock is 23 and for data 22.
I uploaded the example code Simple_test. And when I type something with the keyboard it detects like when I press key 'a' then it prints 'a' on the serial monitor but many times when I do this little quickly by pressing different keys on the keyboard now ESP prints random garbage values on the serial monitor. For example, pressing 'g' on a keyboard and will show '2' on the serial monitor, and pressing the same key a second time shows 'g' properly.

Initially, I thought the logic converter I build had some issues so I tried the same code with Arduino UNO connected to it and it worked perfectly as expected so it's not a hardware issue.

Then I looked in the library code and get to know that it is using an interrupt on the clock pin and as I have been in a similar situation with ESP32 before while using interrupt I finally got to know why this thing is happening.

In Arduino ESP32 support there is some bug for an ESP32 interrupt routine.
When we declare interrupt on falling edge in ESP32 with Arduino IDE with our ISR function. Then ideally the ISR function should get only triggered on the falling edge but when I checked with my logic analyzer it sometimes gets triggers on the rising edge also don't know the reason for that and also after such updates this thing is still not resolved.
Have a look at this issue

So the simple solution for this problem can be implemented by adding one more if condition inside our ISR function like this.

void ps2interrupt(void)
 {
  if(digitalRead(irq_num_pin) == LOW)
  {

This will verify that the pin is in the LOW state or not by just adding this condition in .cpp file the ESP32 garbage problem was fixed and now I am getting perfect readings with my ESP32.

I defined a new variable as irq_num_pin which is equal to the clock pin called in begin function.

So I suggest updating this little logic so that it works better with an ESP32. Also, I have not tested it with Arduino UNO after making this change but I am sure it will not make any difference for other boards.

@techpaul
Copy link
Owner

I will look at incorporating this into this and associated libraries in a few days (other important things first)., more likely as conditional compile for simplicity of support.

Suspect the ESP32 ISR edges issue is a bug in that core as I doubt a micro does not have one edge OR both edges capability.

Thanks for the heads up

@n6il
Copy link

n6il commented Jun 22, 2021

I'm working with ESP32 as well and discovered the same thing just last night.

It might be easier to use this solution. Since you want the FALLING edge, if you get in the ISR and the current level is HIGH then reject it:

void ps2interrupt( void )
{
#ifdef ESP32
if(digitalRead(PS2_IrqPin))
   return;
#endif

@dharmik768
Copy link
Author

dharmik768 commented Jun 22, 2021

Yes, michaels logic will be more efficiente as it targets ESP32 particularly, to fix this interrupt issue.

@techpaul
Copy link
Owner

That is a similar technique I would use and needs to be looked at for a few libraries with user supplied ESP32 support.

Generally prefer the early return on data to ignore or error conditions, less indenting and having to match blocks makes it easier to maintain and more readable

@techpaul
Copy link
Owner

As ESP32 is not one of the listed support, I perhaps need more testing from yourselves as other architecture based defines are set according to board support. Especially when writing to LEDs on keyboard due to interrupts being triggered inside an ISR when sending.

This suggests you are ignoring the compilation WARNING (always correct ALL warnings)

#warning Library is NOT supported on this board Use at your OWN risk

@techpaul
Copy link
Owner

When I get time I will put up a test version for ESP32 when I know what some of the board support parameters like ARDUINO_ARCH_????? in order to incorporate it.

This test branch will need testing by users with the boards BEFORE public release.

@techpaul
Copy link
Owner

I am looking at this issue and I personally do not like this workaround as this is a SILICON issue with the mcu and any spurious interrupts should be handled by arduino-esp32. This SILICON issue is detailed in their own errata document

https://www.espressif.com/sites/default/files/documentation/eco_and_workarounds_for_bugs_in_esp32_en.pdf
"3.14. The ESP32 GPIO peripheral may not trigger interrupts correctly.
Details:
The ESP32 GPIO peripheral may not trigger interrupts correctly if multiple GPIO pads are
configured with edge-triggered interrupts."

The workarounds should have been in the interrupt dispatcher for arduino-esp32

I am working on a better solution for the library

@techpaul
Copy link
Owner

Pre-release candidate V1.09 is available for testing, tell me of any issues
https://github.com/techpaul/PS2KeyAdvanced/releases/tag/V1.0.9
Unzip into the libraries folder to update if you do not have fork pull ability

Some documentation has been updated as well

@techpaul
Copy link
Owner

techpaul commented Aug 6, 2021

This has been realeased as V1.0.9 so closing issue

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

No branches or pull requests

3 participants