Update Keyboard .cpp to include usable begin and end functions

[dragonerboom]

implemented Keyboard.begin() and Keyboard.end() with the simple use of a boolean. Sincirely, Dragonerboom

[CLAassistant]

All committers have signed the CLA.

[github-actions]

Memory usage change @ 44ea612

Board	flash	%	RAM for global variables	%
arduino:avr:leonardo	🔺 +12 - +12	+0.04 - +0.04	🔺 +1 - +1	+0.04 - +0.04
arduino:sam:arduino_due_x_dbg	🔺 +80 - +80	+0.02 - +0.02	N/A	N/A
arduino:samd:mkrzero	🔺 +64 - +64	+0.02 - +0.02	🔺 +4 - +4	+0.01 - +0.01

Click for full report table

Board	examples/Serial flash	%	examples/Serial RAM for global variables	%
arduino:avr:leonardo	12	0.04	1	0.04
arduino:sam:arduino_due_x_dbg	80	0.02	N/A	N/A
arduino:samd:mkrzero	64	0.02	4	0.01

Click for full report CSV

Board,examples/Serial<br>flash,%,examples/Serial<br>RAM for global variables,%
arduino:avr:leonardo,12,0.04,1,0.04
arduino:sam:arduino_due_x_dbg,80,0.02,N/A,N/A
arduino:samd:mkrzero,64,0.02,4,0.01

[edgar-bonet]

Thanks for this pull request! I have to admit I do not understand what is its purpose. What problem is this trying to solve? Do you have a use case?

My understanding is that begin() and end() were never intended to be “switches” to turn the library on and off. My assumption has always been that they are only meant to future-proof the API: if, at some point in the future, the library needs to do some extra initialization and clean-up, this can be done in begin() and end(), without needing to upgrade the library's API.

Given that this library aims at being lightweight, every extra feature should be worth its cost in flash space, even if it is a small cost. I am not yet convinced this feature is worth it.

[edgar-bonet]

This should be static, otherwise it could collide with a user-defined variable. Also, the past participle “enabled” would make more sense than the imperative “enable”: verbs at the imperative are usually used to name functions and methods, not booleans.

[dragonerboom]

done

[edgar-bonet]

For style consistency, there should be a space after the if, and another one before the opening brace.

[edgar-bonet]

Bad indentation. If you want to put everything within an if block, the whole thing should be indented one level deeper. Although it would be far better to avoid this extra nesting and instead return early, as in:

    if (!enable) {
        return 0;
    }

[edgar-bonet]

GCC tells me:

In member function size_t Keyboard_::press(uint8_t):
warning: control reaches end of non-void function [-Wreturn-type]

Same warning on release() and the two overload of write().

[edgar-bonet]

Given that write() only calls press() and release(), and these methods are already doing the test, there is no point in doing it also here.

[edgar-bonet]

Same as before: redundant test.

[dragonerboom]

Oh ok I thought that the begin and end functions were for enabling/disabling the library's functions.But if that's not the case then i believe they should be named shomething like Keyboard.init().