Bangle.js2: Recycled touch event position with long running timeouts about espruino HOT 10 CLOSED

I think I found a fix. My suspicion is this is caused by skipped drag events under high load so the lastTouchX/Y are not updated correctly.

from espruino.

In case that isn't the optimal solution, you could also switch the order of if (bangleTasks & JSBT_DRAG) { and if (bangleTasks & JSBT_TOUCH_MASK) { so that if both events occur on the same frame, the lastTouchX is recent.

Just a thought.

By the way , @gfwilliams could you shed light on this:

Does it mean that all touch events will trigger drag, I assume this code intent, because the touch event uses lastTouchX, which is only set after the DRAG job is performed:
https://github.com/espruino/Espruino/blob/4402d273e9154108108ccd2d85e6f5fc681fbb4b/libs/banglejs/jswrap_bangle.c#L4177C1-L4191

#ifdef TOUCH_DEVICE
  if (bangleTasks & JSBT_DRAG) {
    JsVar *o = jsvNewObject();
    jsvObjectSetChildAndUnLock(o, "x", jsvNewFromInteger(touchX));
    jsvObjectSetChildAndUnLock(o, "y", jsvNewFromInteger(touchY));
    jsvObjectSetChildAndUnLock(o, "b", jsvNewFromInteger(touchPts));
    jsvObjectSetChildAndUnLock(o, "dx", jsvNewFromInteger(lastTouchPts ? touchX-lastTouchX : 0));
    jsvObjectSetChildAndUnLock(o, "dy", jsvNewFromInteger(lastTouchPts ? touchY-lastTouchY : 0));
    jsiQueueObjectCallbacks(bangle, JS_EVENT_PREFIX"drag", &o, 1);
    jsvUnLock(o);
    lastTouchX = touchX;
    lastTouchY = touchY;
    lastTouchPts = touchPts;
  }
#endif

If this variable : lastTouchPts = touchPts; represents if a finger is currently touching or not. Does :
https://github.com/espruino/Espruino/blob/4402d273e9154108108ccd2d85e6f5fc681fbb4b/libs/banglejs/jswrap_bangle.c#L1796C3-L1802

void touchHandler(bool state, IOEventFlags flags) {
  if (state) return; // only interested in when low
  // Ok, now get touch info
  unsigned char buf[6];
  buf[0]=1;
  jsi2cWrite(TOUCH_I2C, TOUCH_ADDR, 1, buf, false);
  jsi2cRead(TOUCH_I2C, TOUCH_ADDR, 6, buf, true);

"returning when state is low" have anything related to the state of touching? like an ordinary button or is it different because its the touch, so its just saying a data is ready to be read? If it were related to the state of touching then it would interfere negatively with the use of touchPts i believe.

What is your thought on why lastTouchX was used on the touch execute, when its set by the drag execute?

Finally, why is jshHadEvent() only called inside the drag condition area?

I predict that your answer will be that the code inside where DRAG task created is the code for all touch events, that makes the most sense to me. I'm just triple checking.

from espruino.

Thanks for this @halemmerich!

It sure looks like touch should use the current touch coordinates - I'm not sure I understand why it didn't. Maybe it's a hangover from before touchX was added for drag handling.

Does it mean that all touch events will trigger drag

Yes - the idea was that drag was fired basically whenever a finger is on (or lifted) so you don't need two different events - and you can use dx/dy to figure out how much actual dragging there was.

does "returning when state is low" have anything related to the state of touching?

No - that's the state of the IRQ line from the touchscreen controller, so it's just low when data is ready.

Finally, why is jshHadEvent() only called inside the drag condition area?

It is called when the interpreter should wake from sleep and handle an event. If there isn't an event it shouldn't handle it - which jshHadEvent did you mean? The one in

Closing this now as PR is merged

from espruino.

Thanks - Well, that's a really odd one. Can reproduce here, but I have no idea why it's happening

from espruino.

Isn't this expected behaviour for a loop? Also what is the notion of 'slots'?

from espruino.

This is not really a loop but rather a sequence of load and idle. The wait constant adds additional idle time by waiting longer before running the next load sequence. That idle time is what I called a slot for event processing.

from espruino.

Is it reproducible on the emulator? I mean the original issue.

from espruino.

I still don't understand why the code allows a bangleTasks bit to be set whilst allowing to not get to line 1789 and call jshHadEvent() , what am I missing?

If the jshHadEvent() is not called, do the bits set on bangleTasks get discard/reset?

from espruino.

Ahh, ok, I see what you mean.

Yes, if bangleTasks is set then so should HadEvent, so that looks like a bug - although I'm not sure if it actually shows itself since the touchscreen reports a bunch of events in quick succession, one of which probably fires jshHadEvent anyway. It could do with fixing either way though - I'll get something in for it

from espruino.

Ahh, ok, I see what you mean.

Yes, if bangleTasks is set then so should HadEvent, so that looks like a bug - although I'm not sure if it actually shows itself since the touchscreen reports a bunch of events in quick succession, one of which probably fires jshHadEvent anyway. It could do with fixing either way though - I'll get something in for it

Yes its confusing to read, but has little effect on operation, possibly even 0. If it works, it works, but is always nice to have code that makes more intuitive sense.

edit: ye so the bug would be when single tap in same location, 2nd finger maybe + jshEvent() somewhere else, some rare bug, very small. possibly some timing issue created, with a delayed touch somewhere, if allowed to sleep with a task.

from espruino.