[RFC 5/11] Linux FF: Linux effect parameters: time parameters

[Elias Vanderstuyft]

a)  Because of the higher time resolution of DInput parameters (in
microseconds instead of milliseconds), e.g. dwPeriod,
    and because truncating may result in "periodic.period == 0", while
"1 <= dwPeriod (non-zero) < 1000",
    and because "periodic.period == 0" is an invalid argument,
    we might set periodic.period to the smallest non-zero value (that
is 1) when dwPeriod is non-zero, to avoid unnecessary exceptions.
    So change in dinput/effect_linuxinput.c:488 :
        This->effect.u.periodic.period = tsp->dwPeriod / 1000;
    to :
        if (0 < tsp->dwPeriod && tsp->dwPeriod <= 1000)
            This->effect.u.periodic.period = 1;
        else
            This->effect.u.periodic.period = tsp->dwPeriod / 1000;


b)  Anticipate if dwPeriod < 0, otherwise periodic.period will
incorrectly wrap around because it's unsigned.
    We can choose to let periodic.period be proportional to:
    - 0    (driver will generate EINVAL)
    - abs(dwPeriod)    (no error, and is mathematically more correct)

    Choosing the latter, so change in dinput/effect_linuxinput.c:488 :
        if (0 < tsp->dwPeriod && tsp->dwPeriod <= 1000)
            This->effect.u.periodic.period = 1;
        else
            This->effect.u.periodic.period = tsp->dwPeriod / 1000;
    to :
        if (tsp->dwPeriod && abs(tsp->dwPeriod) <= 1000)
            This->effect.u.periodic.period = 1;
        else
            This->effect.u.periodic.period = abs(tsp->dwPeriod) / 1000;


c)  Something similar is needed for 'dwDuration':
    Set infinite replay-length in linux by "replay.length == 0":
discussion: http://www.mail-archive.com/linux-input@vger.kernel.org/msg08459.html
    Knowing that DInput "INFINITE == -1", we convert -1 to 0.
    If "peff->dwDuration == 0", we can:
    - not start the effect
    - set the effect duration to the absolute minimum

    I would choose the latter, because otherwise more behaviour of the
code would change.
    And lastly, we don't throw exceptions when "dwDuration < 0": MSDN
does not require us to throw DIERR_INVALIDPARAM.
    So, change in dinput/effect_linuxinput.c:191 :
        if (dwFlags & DIEP_DURATION) {
            peff->dwDuration = (DWORD)This->effect.replay.length * 1000;
        }
    to :
        if (dwFlags & DIEP_DURATION) {
            peff->dwDuration = (This->effect.replay.length ?
(DWORD)This->effect.replay.length * 1000 : -1);
        }

    And change in dinput/effect_linuxinput.c:424 :
        if (dwFlags & DIEP_DURATION)
            This->effect.replay.length = peff->dwDuration / 1000;
    to :
        if (dwFlags & DIEP_DURATION) {
            if (peff->dwDuration == -1)
                This->effect.replay.length = 0; // Infinity
            else if (peff->dwDuration > 1000)
                This->effect.replay.length = peff->dwDuration / 1000;
            else
                This->effect.replay.length = 1; // Smallest non-zero value
        }


Elias
-------------- next part --------------
/////////////////////////////    Linux effect parameters: time parameters    /////////////////////////////

a)  Because of the higher time resolution of DInput parameters (in microseconds instead of milliseconds), e.g. dwPeriod,
    and because truncating may result in "periodic.period == 0", while "1 <= dwPeriod (non-zero) < 1000",
    and because "periodic.period == 0" is an invalid argument,
    we might set periodic.period to the smallest non-zero value (that is 1) when dwPeriod is non-zero, to avoid unnecessary exceptions.
    So change in dinput/effect_linuxinput.c:488 :
        This->effect.u.periodic.period = tsp->dwPeriod / 1000;
    to :
        if (0 < tsp->dwPeriod && tsp->dwPeriod <= 1000)
            This->effect.u.periodic.period = 1;
        else
            This->effect.u.periodic.period = tsp->dwPeriod / 1000;


b)  Anticipate if dwPeriod < 0, otherwise periodic.period will incorrectly wrap around because it's unsigned.
    We can choose to let periodic.period be proportional to:
    - 0    (driver will generate EINVAL)
    - abs(dwPeriod)    (no error, and is mathematically more correct)
    
    Choosing the latter, so change in dinput/effect_linuxinput.c:488 :
        if (0 < tsp->dwPeriod && tsp->dwPeriod <= 1000)
            This->effect.u.periodic.period = 1;
        else
            This->effect.u.periodic.period = tsp->dwPeriod / 1000;
    to :
        if (tsp->dwPeriod && abs(tsp->dwPeriod) <= 1000)
            This->effect.u.periodic.period = 1;
        else
            This->effect.u.periodic.period = abs(tsp->dwPeriod) / 1000;


c)  Something similar is needed for 'dwDuration':
    Set infinite replay-length in linux by "replay.length == 0": discussion: http://www.mail-archive.com/linux-input@vger.kernel.org/msg08459.html
    Knowing that DInput "INFINITE == -1", we convert -1 to 0.
    If "peff->dwDuration == 0", we can:
    - not start the effect
    - set the effect duration to the absolute minimum
    
    I would choose the latter, because otherwise more behaviour of the code would change.
    And lastly, we don't throw exceptions when "dwDuration < 0": MSDN does not require us to throw DIERR_INVALIDPARAM.
    So, change in dinput/effect_linuxinput.c:191 :
        if (dwFlags & DIEP_DURATION) {
            peff->dwDuration = (DWORD)This->effect.replay.length * 1000;
        }
    to :
        if (dwFlags & DIEP_DURATION) {
            peff->dwDuration = (This->effect.replay.length ? (DWORD)This->effect.replay.length * 1000 : -1);
        }
    
    And change in dinput/effect_linuxinput.c:424 :
        if (dwFlags & DIEP_DURATION)
            This->effect.replay.length = peff->dwDuration / 1000;
    to :
        if (dwFlags & DIEP_DURATION) {
            if (peff->dwDuration == -1)
                This->effect.replay.length = 0; // Infinity
            else if (peff->dwDuration > 1000)
                This->effect.replay.length = peff->dwDuration / 1000;
            else
                This->effect.replay.length = 1; // Smallest non-zero value
        }