Hi Hans,

pos_x[] is not non-inialized,  it may be previous pos_x, it is still ok for
getting dx.
"n" var is trying to reduce "untouch" sensor data for post proccessing. I
attach a new patch
may be more clear, also fixed unexpected movement when button status or
ntouch changing.

Cheers,

Huang Wen Hui

2014-01-30 Hans Petter Selasky <h...@bitfrost.no>:

> Hi Huang,
>
>
> On 01/30/14 06:56, Huang Wen Hui wrote:
>
>> Hans,
>>
>> Thanks for you take care of it and commit it! I found two problems:
>>
>> 1. The selection is not expected when selection with 2 fingers sometimes.
>> 2.  Unexpected scrolling when Click with 2 fingers.
>>
>> This patch can fix that. The var "n" modify to "ntouch"  seems to be
>> necessary.
>>
>>
> Right, but aren't we then accessing non-initialised sc->pos_x[] data ?
>
> Because if ntouch == 2, n can be less than or equal to 2, due to continue
> in for-loop above. What is the purpose of the "n" variable?
>
> Can you explain?
>
> -               if (n == 2) {
> +               if (ntouch == 2) {
>                         sc->distance = max(sc->distance, max(
>                             abs(sc->pos_x[0] - sc->pos_x[1]),
>                             abs(sc->pos_y[0] - sc->pos_y[1])));
>
> --HPS
>
>
>> Cheers,
>> Huang Wen Hui
>>
>>
>> 2014-01-29 Hans Petter Selasky <h...@bitfrost.no>
>>
>>  On 01/29/14 09:49, Lundberg, Johannes wrote:
>>>
>>>  Hi
>>>>
>>>> I tested the driver on a 2012 Macbook Air 11" and it works great! Good
>>>> job!
>>>>
>>>> Is there a way to disable click-by-touch? I always preferred clicking
>>>> with
>>>> the physical button that is built in to the pad.
>>>>
>>>>
>>>>  Hi,
>>>
>>> I've added an "#if 0" around the 1 finger tap code until further. Maybe
>>> this feature can be tunable?
>>>
>>> I fixed the code style, added some range checks and cleared some buffer
>>> issues.
>>>
>>> When you assign a signed value to an unsigned variable, you should range
>>> check it, because the sign might cause an overflow when you use it later
>>> on.
>>>
>>> int8_t x = -1;
>>>
>>> uint32_t t = x;
>>>
>>> "t" is now "0xffffffffU" and not "255".
>>>
>>> Tested the code on my MacBookPro. Hope I didn't break anything. If so,
>>> send a patch to freebsd-usb.
>>>
>>> http://svnweb.freebsd.org/changeset/base/261260
>>>
>>> To get the touchpad working with Xorg, I needed to re-compile HALD with
>>> the attached patch.
>>>
>>> kwm: Can you get the attached patch into ports?
>>>
>>> Auto-loading of wsp via devd will be done later. Simply need to
>>> re-generate usb.conf in /etc ...
>>>
>>> --HPS
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> freebsd-curr...@freebsd.org mailing list
>>> http://lists.freebsd.org/mailman/listinfo/freebsd-current
>>> To unsubscribe, send any mail to "freebsd-current-unsubscribe@
>>> freebsd.org"
>>>
>>
>
--- wsp.c.orig  2014-01-30 08:14:26.000000000 +0800
+++ wsp.c       2014-01-31 09:44:02.000000000 +0800
@@ -605,7 +605,8 @@
        int     dz_count;
 #define        WSP_DZ_MAX_COUNT        32
        int     dt_sum;                 /* T-axis cumulative movement */
-
+       
+       uint8_t o_ntouch;               /* old touch finger status */
        uint8_t finger;                 /* 0 or 1 *, check which finger moving 
*/
        uint16_t intr_count;
 #define        WSP_TAP_THRESHOLD       3
@@ -871,7 +872,6 @@
        int dx = 0;
        int dy = 0;
        int dz = 0;
-       int n = 0;
        int len;
        int i;
 
@@ -936,13 +936,9 @@
                            f[i].tool_major, f[i].tool_minor, f[i].orientation,
                            f[i].touch_major, f[i].touch_minor, f[i].multi);
 
-                       if (f[i].touch_major < tun.pressure_untouch_threshold)
-                               continue;
-
-                       sc->pos_x[n] = f[i].abs_x;
-                       sc->pos_y[n] = params->y.min + params->y.max - 
f[i].abs_y;
-                       sc->index[n] = &f[i];
-                       n++;
+                       sc->pos_x[i] = f[i].abs_x;
+                       sc->pos_y[i] = params->y.min + params->y.max - 
f[i].abs_y;
+                       sc->index[i] = &f[i];
                }
 
                sc->sc_status.flags &= ~MOUSE_POSCHANGED;
@@ -957,8 +953,8 @@
                if (h->q2 == 4)
                        sc->intr_count++;
 
-               if (sc->ntaps < n) {
-                       switch (n) {
+               if (sc->ntaps < ntouch) {
+                       switch (ntouch) {
                        case 1:
                                if (f[0].touch_major > 
tun.pressure_tap_threshold)
                                        sc->ntaps = 1;
@@ -978,7 +974,7 @@
                                break;
                        }
                }
-               if (n == 2) {
+               if (ntouch == 2) {
                        sc->distance = max(sc->distance, max(
                            abs(sc->pos_x[0] - sc->pos_x[1]),
                            abs(sc->pos_y[0] - sc->pos_y[1])));
@@ -1050,15 +1046,33 @@
                        if (sc->sc_touch == WSP_SECOND_TOUCH)
                                sc->sc_touch = WSP_TOUCHING;
 
-                       if (n != 0 &&
+                       if (ntouch != 0 &&
                            h->q2 == 4 &&
                            f[0].touch_major >= tun.pressure_touch_threshold) {
                                dx = sc->pos_x[0] - sc->pre_pos_x;
                                dy = sc->pos_y[0] - sc->pre_pos_y;
-                               if (n == 2 && sc->sc_status.button != 0) {
+
+                               /* Ignore movement from ibt=1 to ibt=0 */
+                               if (sc->sc_status.obutton != 0 && 
+                                   sc->sc_status.button == 0) {
+                                       dx = 0;
+                                       dy = 0;
+                               }
+                               /* Ignore movement if ntouch changed */
+                               if (sc->o_ntouch != ntouch) {
+                                       dx = 0;
+                                       dy = 0;
+                               }
+
+                               if (ntouch == 2 && sc->sc_status.button != 0) {
                                        dx = sc->pos_x[sc->finger] - 
sc->pre_pos_x;
                                        dy = sc->pos_y[sc->finger] - 
sc->pre_pos_y;
-                                       if (f[0].origin == 0 || f[1].origin == 
0) {
+                                       
+                                       /* Ignore movement of switch finger or
+                                        * movement from ibt=0 to ibt=1
+                                         */
+                                       if (f[0].origin == 0 || f[1].origin == 
0 ||
+                                           sc->sc_status.obutton != 
sc->sc_status.button) {
                                                dx = 0;
                                                dy = 0;
                                                sc->finger = 0;
@@ -1092,7 +1106,7 @@
                        sc->dx_sum += dx;
                        sc->dy_sum += dy;
 
-                       if (n == 2 && sc->sc_status.button == 0) {
+                       if (ntouch == 2 && sc->sc_status.button == 0) {
                                if (sc->scr_mode == WSP_SCR_NONE &&
                                    abs(sc->dx_sum) + abs(sc->dy_sum) > 50)
                                        sc->scr_mode = abs(sc->dx_sum) >
@@ -1134,10 +1148,12 @@
                sc->pre_pos_x = sc->pos_x[0];
                sc->pre_pos_y = sc->pos_y[0];
 
-               if (n == 2 && sc->sc_status.button != 0) {
+               if (ntouch == 2 && sc->sc_status.button != 0) {
                        sc->pre_pos_x = sc->pos_x[sc->finger];
                        sc->pre_pos_y = sc->pos_y[sc->finger];
                }
+               sc->o_ntouch = ntouch;
+
        case USB_ST_SETUP:
 tr_setup:
                /* check if we can put more data into the FIFO */
_______________________________________________
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"

Reply via email to