arduino-libraries / Keyboard

GNU Lesser General Public License v3.0
231 stars 160 forks source link

Keyboard.println unexpected behaviour #13

Closed per1234 closed 6 years ago

per1234 commented 6 years ago

Moved from arduino/Arduino#6922 by @sterretje

When using Keyboard.println, one expects the cursor to go to beginning of the next line in the active window. This does not happen,

The description in the reference section states the below and technically that is correct

Sends a keystroke to a connected computer, followed by a newline and carriage return.

A note on the reference page would be useful. Workaround is to use Keyboard.print{"Hallo") followed by Keyboard.write(KEY_RETURN).

A bugfix would even be better ;-)

In case relevant: Windows 8 64 bit, IDE 1.6.6 (so you can determine library version), tested with Notepad and Notepad++.

PaulStoffregen commented 6 years ago

FWIW, this is how Keyboard.println() has long worked on Teensy, ~3 years before Arduino made Leonardo.

The implementation is very simple. Teensy's Keyboard.write() function ignores 13 and uses 1 line of code to translate 10 to KEY_ENTER. Simple stuff.

sterretje commented 6 years ago

I have commented this a well in https://github.com/arduino-libraries/Keyboard/commit/977e7b494fa430e4f63af7b6dd34e31db884f261

I might not understand this very well but write is supposed to return the number of bytes written. If you don't write anything, why return 13 ('\r')?

mjbcopland commented 6 years ago

Can anyone confirm whether this issue is actually resolved? Looking at the code I don't understand how, but unfortunately don't have the hardware to hand for testing.

I think 977e7b494fa430e4f63af7b6dd34e31db884f261 accidentally fixed this issue, then I think 87c2941ab18dcf83046745132cdd3341db320c89 reopened it.

Keyboard inherits from Print, which returns early if a call to write() returns 0. What was happening before is that '\r' translates to 0 in the ACSII map, so Keyboard::press() was returning 0, which propagates up to Keyboard::write(), then to Print::write(), which exits the loop before the '\n' is reached.

Handling '\r' by returning it inadvertently fixed this issue because it's non-zero, so Print::write() continues with the remaining characters. If we return 0 instead then we're back where we started.

Furthermore, '\n' already has an appropriate value in the ASCII map; adding an explicit check to set it to KEY_ENTER should not have changed anything, as it should not have been broken in the first place.

Surely a simple Keyboard.print("\r\n") compared to Keyboard.print("\n") will demonstrate this? Can anyone check? If I'm right then #15 should fix it.

facchinm commented 6 years ago

@mjbcopland thanks for finding the root cause. I'm cherrypicking your patch from #15 adding a couple of minor fixing (it was missing the declaration and a missing ; )