Dear Thorsten, dear MIDIBOXers
First of all Happy New Year 2015!
During regression tests of the midibox_kb_v1 project I found a nasty bug in
SVN.MIOS32\trunk\modules\keyboard\keyboard.c at the KEYBOARD_NotifyToggle() fct.:
// released key ?
if( depressed ) {
.....
[L578]: s16 delay = *ts_break_ptr - *ts_make_ptr;
.....
and
} else {
// pressed key ?
.....
[L631]: s16 delay = *ts_make_ptr - *ts_break_ptr;
.....
}
the (signed short) variable s16 delay leads to the following behaviour during delay calculations in case of overrun
(s. 2nd & 3rd line during a debug session):
[18721.553] Released: s16 delay= 17982 = u16 ts_break=58069 - u16 ts_make =40087
[18796.922] Pressed : s16 delay= 24956 = u16 ts_make =14748 - u16 ts_break=55328
[18817.231] Released: s16 delay=-20449 = u16 ts_break=40161 - u16 ts_make =60610 /!\
:sad:
the 2nd line is still correct because the subtraction doesn´t exceed the positive range of the (signed short) s16 delay range.
the 3rd line is mathematically correct, but not from the applicational view point.
The local fct.:
[L802]: static int KEYBOARD_GetVelocity(s16 delay, u16 delay_slowest, u16 delay_fastest)
doesn´t handle negative delay values correctly because it handles all values below delay_fastest as max. velocity!
So I changed the delay values in line 578, 631 & 802 from s16 to u16 with the result that the overrun case is now handled correctly:
Released: u16 delay= 17982 = u16 ts_break=58069 - u16 ts_make =40087
Pressed : u16 delay= 24956 = u16 ts_make =14748 - u16 ts_break=55328
Released: u16 delay= 45087 = u16 ts_break=40161 - u16 ts_make =60610
:rolleyes:
Thorsten, would you be so kind to check if my findings are correct, and make the proposed changes in
modules/keyboard/keyboard.c and in apps/tutorials/029_keyboard_velocity/app.c if you confirm.
Thanks and best regards,
Jo