Fix a few STMPE11 touchscreen and NxWM touchscreen calibration bugs

git-svn-id: https://nuttx.svn.sourceforge.net/svnroot/nuttx/trunk@4723 7fd9a85b-ad96-42d3-883c-3090e2eb8679

NxWidgets/ChangeLog.txt

@@ -46,12 +46,17 @@
   external logic.
 * CWindowEventHandler, CWindowEventHandlerList, CWidgetControl: New
   callback classes to receive notifications about window events.
-* CFullScreenWindow and CTaskbar:  Add support in NxWM for full screen
+* NxWM::CFullScreenWindow and NxWM::CTaskbar:  Add support in NxWM for full
-  window applications.
+  screen window applications.
 * All application windows now use CWindowEventHandler and CWindowEventHandlerList
   to get notifications about mouse and keyboard events.  These class will
   then automatically handle polling (with no need for a modal loop).
-* CTouchscreen and CCalibration: Add touchscreen support (still a long way
+* NxWM::CTouchscreen and NxWM::CCalibration: Add touchscreen support (still a long
-  to go).
+  way to go).
+* NxWM::g_playBitmp: Change the play icon again.  These tiny touch icons
+  must be very simple.
+* NxWM::CCalibration: Beef up touch input handling logic.  Now changes the
+  color of the touch circle to yellow when it is touched.
+* NxWM::CTouchscreen: Do not read touchscreen data when there is no consumer.
 
 

NxWidgets/UnitTests/nxwm/main.cxx

@@ -79,16 +79,16 @@
 
 struct SNxWmTest
 {
-  NxWM::CTaskbar     *taskbar;        // The task bar
+  NxWM::CTaskbar     *taskbar;             // The task bar
-  NxWM::CStartWindow *startwindow;    // The start window
+  NxWM::CStartWindow *startwindow;         // The start window
 #ifdef CONFIG_NXWM_TOUCHSCREEN
-  NxWM::CTouchscreen *touchscreen;    // The touchscreen
+  NxWM::CTouchscreen *touchscreen;         // The touchscreen
-  NxWM::CCalibration *calibration;    // The touchscreen calibration application
+  NxWM::CCalibration *calibration;         // The touchscreen calibration application
-  struct NxWM::SCalibrationData data; // Calibration data
+  struct NxWM::SCalibrationData calibData; // Calibration data
 #endif
-  unsigned int        mmInitial;      // Initial memory usage
+  unsigned int        mmInitial;           // Initial memory usage
-  unsigned int        mmStep;         // Memory Usage at beginning of test step
+  unsigned int        mmStep;              // Memory Usage at beginning of test step
-  unsigned int        mmSubStep;      // Memory Usage at beginning of test sub-step
+  unsigned int        mmSubStep;           // Memory Usage at beginning of test sub-step
 };
 
 /////////////////////////////////////////////////////////////////////////////
@@ -459,6 +459,12 @@ static bool createCalibration(void)
 #ifdef CONFIG_NXWM_TOUCHSCREEN
 static bool runCalibration(void)
 {
+  // 1. Start the calibration application
+  // 2. Wait until calibration data is available
+  // 3. Stop the calibration application
+  // 4. Provide the calibration data to the touchscreen device application
+  // 5. Enable forwarding of touchscreen input
+
   // Call CTaskBar::startApplication to start the Calibration application
 
   printf(MAIN_STRING "Start the calibration application\n");
@@ -472,11 +478,12 @@ static bool runCalibration(void)
   // Wait for calibration data
 
   printf(MAIN_STRING "Get calibration data\n");
-  if (!g_nxwmtest.calibration->waitCalibrationData(g_nxwmtest.data))
+  if (!g_nxwmtest.calibration->waitCalibrationData(g_nxwmtest.calibData))
     {
       printf(MAIN_STRING "ERROR: Failed to get calibration data\n");
       return false;
     }
+  showTestCaseMemory("After getting calibration data");
 
   printf(MAIN_STRING "Stop the calibration application\n");
   if (!g_nxwmtest.taskbar->stopApplication(g_nxwmtest.calibration))
@@ -484,8 +491,13 @@ static bool runCalibration(void)
       printf(MAIN_STRING "ERROR: Failed to stop the calibration application\n");
       return false;
     }
-
   showTestCaseMemory("After stopping the calibration application");
+
+  // Configure the touchscreen device and enable touch forwarding
+
+  g_nxwmtest.touchscreen->setCalibrationData(g_nxwmtest.calibData);
+  g_nxwmtest.touchscreen->setEnabled(true);
+  showTestCaseMemory("After givin calibration dato to the touchscreen device\n");
   return true;
 }
 #endif

NxWidgets/nxwm/include/ccalibration.hxx

@@ -133,6 +133,7 @@ namespace NxWM
     struct nxgl_point_s        m_touchPos;        /**< This is the last touch position */
     bool                       m_stop;            /**< True: We have been asked to stop the calibration */
     bool                       m_touched;         /**< True: The screen is touched */
+    uint8_t                    m_touchId;         /**< The ID of the touch */
     sem_t                      m_waitSem;         /**< Supports wait for calibration data */
     struct nxgl_point_s        m_calibData[CALIB_DATA_POINTS];
 

NxWidgets/nxwm/include/ctouchscreen.hxx

@@ -175,17 +175,7 @@ namespace NxWM
      * @param data.  A reference to the touchscreen data.
      */
 
-    inline void setCalibrationData(struct SCalibrationData &caldata)
+    void setCalibrationData(struct SCalibrationData &caldata);
-    {
-      // Save a copy of the calibration data
-
-      m_calibData = caldata;
- 
-      // Note that we have calibration data.  Data will now be scaled and forwarded
-      // to NX (unless we are still in cpature mode)
- 
-      m_calibrated = true;
-    }
 
     /**
      * Capture raw driver data.  This method will capture mode one raw touchscreen

NxWidgets/nxwm/include/nxwmconfig.hxx

@@ -356,8 +356,11 @@
  *   CONFIG_NXWM_DEFAULT_BACKGROUNDCOLOR
  * CONFIG_NXWM_CALIBRATION_LINECOLOR - The color of the lines used in the
  *   touchscreen calibration display.  Default:  MKRGB(0, 0, 128) (dark blue)
- * CONFIG_NXWM_CALIBRATION_BACKGROUNDCOLOR - The background color of the
+ * CONFIG_NXWM_CALIBRATION_CIRCLECOLOR - The color of the circle in the
  *   touchscreen calibration display.  Default:  MKRGB(255, 255, 255) (white)
+ * CONFIG_NXWM_CALIBRATION_CIRCLECOLOR - The color of the circle in the
+ *   touchscreen calibration display after the touch is recorder.  Default: 
+ *   MKRGB(255, 255, 96) (very light yellow)
  * CONFIG_NXWM_CALIBRATION_ICON - The ICON to use for the touchscreen
  *   calibration application.  Default:  NxWM::g_calibrationBitmap
  */
@@ -374,6 +377,10 @@
 #  define CONFIG_NXWM_CALIBRATION_CIRCLECOLOR MKRGB(255, 255, 255)
 #endif
 
+#ifndef CONFIG_NXWM_CALIBRATION_TOUCHEDCOLOR
+#  define CONFIG_NXWM_CALIBRATION_TOUCHEDCOLOR MKRGB(255, 255, 96)
+#endif
+
 #ifndef CONFIG_NXWM_CALIBRATION_ICON
 #  define CONFIG_NXWM_CALIBRATION_ICON NxWM::g_calibrationBitmap
 #endif

NxWidgets/nxwm/src/ccalibration.cxx

@@ -302,7 +302,8 @@ void CCalibration::touchscreenInput(struct touch_sample_s &sample)
       // Yes.. but ignore drag events if we did not see the matching
       // touch down event
 
-      if ((sample.point[0].flags & TOUCH_DOWN) != 0 || m_touched)
+      if ((sample.point[0].flags & TOUCH_DOWN) != 0 ||
+          (m_touched && sample.point[0].id == m_touchId))
         {
           // Yes.. save the touch position and wait for the TOUCH_UP report
 
@@ -314,9 +315,19 @@ void CCalibration::touchscreenInput(struct touch_sample_s &sample)
                 sample.point[0].y,  sample.point[0].h,     sample.point[0].w,
                 sample.point[0].pressure);
 
-          // Remember that we saw the touch down event
+          // Show calibration screen again, changing the color of the circle to
+          // make it clear that the touch has been noticed.
 
-          m_touched    = true;
+          if (!m_touched)
+            {
+              m_screenInfo.circleFillColor = CONFIG_NXWM_CALIBRATION_TOUCHEDCOLOR;
+              showCalibration();
+              m_touched = true;
+            }
+
+          // Remember the ID of the touch down event
+
+          m_touchId = sample.point[0].id;
         }
     }
 
@@ -324,20 +335,32 @@ void CCalibration::touchscreenInput(struct touch_sample_s &sample)
 
   else if ((sample.point[0].flags & TOUCH_UP) != 0)
     {
-      // Yes.. did we see the matching pen down event?
+      // Yes.. did we see the pen down event?
 
       if (m_touched)
         {
-          // Yes.. invoke the state machine.
+          // Yes.. For the matching touch ID?
 
-          gvdbg("State: %d Screen x: %d y: %d  Touch x: %d y: %d\n",
+          if (sample.point[0].id == m_touchId)
-                m_state, m_screenInfo.pos.x, m_screenInfo.pos.y,
+            {
-                m_touchPos.x, m_touchPos.y);
+              // Yes.. invoke the state machine.
 
-          stateMachine();
+              gvdbg("State: %d Screen x: %d y: %d  Touch x: %d y: %d\n",
+                    m_state, m_screenInfo.pos.x, m_screenInfo.pos.y,
+                    m_touchPos.x, m_touchPos.y);
+
+              stateMachine();
+            }
+          else
+            {
+              // No... restore the un-highlighted circle
+ 
+              m_screenInfo.circleFillColor = CONFIG_NXWM_CALIBRATION_CIRCLECOLOR;
+              showCalibration();
+            }
         }
 
-      // In any event, the touch is not down
+      // In any event, the screen is not touched
 
       m_touched = false;
     }

NxWidgets/nxwm/src/ctouchscreen.cxx

@@ -195,6 +195,32 @@ bool CTouchscreen::start(void)
   return m_state == LISTENER_RUNNING;
 }
 
+/**
+ * Provide touchscreen calibration data.  If calibration data is received (and
+ * the touchscreen is enabled), then received touchscreen data will be scaled
+ * using the calibration data and forward to the NX layer which dispatches the
+ * touchscreen events in window-relative positions to the correct NX window.
+ *
+ * @param data.  A reference to the touchscreen data.
+ */
+
+void CTouchscreen::setCalibrationData(struct SCalibrationData &caldata)
+{
+  // Save a copy of the calibration data
+
+  m_calibData = caldata;
+ 
+  // Note that we have calibration data.  Data will now be scaled and forwarded
+  // to NX (unless we are still in cpature mode)
+ 
+   m_calibrated = true;
+
+  // Wake up the listener thread so that it will use our buffer
+  // to receive data
+
+  (void)pthread_kill(m_thread, CONFIG_NXWM_TOUCHSCREEN_SIGNO);
+}
+
 /**
  * Capture raw driver data.  This method will capture mode one raw touchscreen
  * input.  The normal use of this method is for touchscreen calibration.
@@ -284,6 +310,31 @@ FAR void *CTouchscreen::listener(FAR void *arg)
 
   while (This->m_state == LISTENER_RUNNING)
     {
+      // We may be running in one of three states
+      //
+      // 1. Disabled or no calibration data:  In this case, just wait for a signal
+      //    indicating that the state has changed.
+      // 2. Performing calibration and reporting raw touchscreen data
+      // 3. Normal operation, reading touchscreen data and forwarding it to NX
+
+      // Check if we need to collect touchscreen data.  That is, that we are enabled,
+      // AND have calibratation data OR if we need to collect data for the calbration
+      // process.
+
+      while ((!This->m_enabled || !This->m_calibrated) && !This->m_capture)
+        {
+          // No.. just sleep.  This sleep will be awakened by a signal if there
+          // is anything for this thread to do
+
+          sleep(1);
+
+          // We woke up here either because the one second elapsed or because we
+          // were signalled.  In either case we need to check the conditions and
+          // determine what to do next.
+        }
+
+      // We are going to collect a sample..
+      //
       // The sample pointer can change dynamically let's sample it once
       // and stick with that pointer.
 
@@ -381,7 +432,8 @@ void CTouchscreen::handleMouseInput(struct touch_sample_s *sample)
 
   if (!m_enabled || !m_calibrated)
     {
-      // No.. we are not yet ready to process touchscreen data
+      // No.. we are not yet ready to process touchscreen data (We don't
+      // really every get to this condition.
 
       return;
     }

nuttx/ChangeLog

@@ -2735,4 +2735,9 @@
 	* drivers/input/stmpe11_tsc.c:  Fix some status checks so that the touchscreen
 	  interrupt handling logic does not read data if the fifo is not at the
 	  threshold level.
-
+	* include/nuttx/wqueue.h: Add macro work_available() to determine if the
+	  previously scheduled work has completed.
+	* drivers/stmpe11_tsc.c:  Correct errors: (1) Since all interrupt logic is done on
+	  the worker thread, disabling interrupts does not provide protected; Need to
+	  disable pre-emption.  (2) Fix handling of touch ID and (2) add some logic to
+	  prevent certain kinds of data overrun.

nuttx/drivers/input/stmpe11_base.c

@@ -188,7 +188,7 @@ static int stmpe11_interrupt(int irq, FAR void *context)
    * to protected the work queue.
    */
 
-  DEBUGASSERT(priv->work.worker == NULL);
+  DEBUGASSERT(work_available(&priv->work));
   ret = work_queue(&priv->work, stmpe11_worker, priv, 0);
   if (ret != 0)
     {

nuttx/drivers/input/stmpe11_tsc.c

@@ -210,22 +210,18 @@ static void stmpe11_notify(FAR struct stmpe11_dev_s *priv)
  *   Check if touchscreen sample data is available now and, if so, return
  *   the sample data.  This is part of the stmpe11_read logic.
  *
+ * Assumption:
+ *   Pre-emption is disable to prevent the worker thread from running.
+ *   Otherwise, sampled data may continue to change.
+ *
  ****************************************************************************/
 
 static int stmpe11_sample(FAR struct stmpe11_dev_s *priv,
                           FAR struct stmpe11_sample_s *sample)
 {
-  irqstate_t flags;
   int ret = -EAGAIN;
 
-  /* Interrupts me be disabled when this is called to (1) prevent posting
+  /* Is there new STMPE11 sample data available? */
-   * of semphores from interrupt handlers, and (2) to prevent sampled data
-   * from changing until it has been reported.
-   */
-
-  flags = irqsave();
-
-  /* Is there new TSC2007 sample data available? */
 
   if (priv->penchange)
     {
@@ -263,7 +259,6 @@ static int stmpe11_sample(FAR struct stmpe11_dev_s *priv,
       ret = OK;
     }
 
-  irqrestore(flags);
   return ret;
 }
 
@@ -279,19 +274,13 @@ static int stmpe11_sample(FAR struct stmpe11_dev_s *priv,
 static inline int stmpe11_waitsample(FAR struct stmpe11_dev_s *priv,
                                      FAR struct stmpe11_sample_s *sample)
 {
-  irqstate_t flags;
   int ret;
 
-  /* Interrupts me be disabled when this is called to (1) prevent posting
+  /* Disable pre-emption to prevent the worker thread from running
-   * of semphores from interrupt handlers, and (2) to prevent sampled data
+   * asynchronously.
-   * from changing until it has been reported.
-   *
-   * In addition, we will also disable pre-emption to prevent other threads
-   * from getting control while we muck with the semaphores.
    */
 
   sched_lock();
-  flags = irqsave();
 
   /* Now release the semaphore that manages mutually exclusive access to
    * the device structure.  This may cause other tasks to become ready to
@@ -312,8 +301,11 @@ static inline int stmpe11_waitsample(FAR struct stmpe11_dev_s *priv,
       ret = sem_wait(&priv->waitsem);
       priv->nwaiters--;
 
+      /* When we are re-awakened, pre-emption will again be disabled */
+
       if (ret < 0)
         {
+#ifdef CONFIG_DEBUG
           // Sample the errno (debug output could change it)
 
           int errval = errno;
@@ -324,6 +316,7 @@ static inline int stmpe11_waitsample(FAR struct stmpe11_dev_s *priv,
 
           idbg("sem_wait failed: %d\n", errval);
           DEBUGASSERT(errval == EINTR);
+#endif
           ret = -EINTR;
           goto errout;
         }
@@ -337,13 +330,6 @@ static inline int stmpe11_waitsample(FAR struct stmpe11_dev_s *priv,
   ret = sem_wait(&priv->exclsem);
 
 errout:
-  /* Then re-enable interrupts.  We might get interrupt here and there
-   * could be a new sample.  But no new threads will run because we still
-   * have pre-emption disabled.
-   */
-
-  irqrestore(flags);
-
   /* Restore pre-emption.  We might get suspended here but that is okay
    * because we already have our sample.  Note:  this means that if there
    * were two threads reading from the STMPE11 for some reason, the data
@@ -542,7 +528,7 @@ static ssize_t stmpe11_read(FAR struct file *filep, FAR char *buffer, size_t len
   report = (FAR struct touch_sample_s *)buffer;
   memset(report, 0, SIZEOF_TOUCH_SAMPLE_S(1));
   report->npoints            = 1;
-  report->point[0].id        = priv->id;
+  report->point[0].id        = sample.id;
   report->point[0].x         = sample.x;
   report->point[0].y         = sample.y;
   report->point[0].pressure  = sample.z;
@@ -962,7 +948,7 @@ void stmpe11_tscworker(FAR struct stmpe11_dev_s *priv, uint8_t intsta)
 
   else if ((intsta & (INT_FIFO_TH|INT_FIFO_OFLOW)) != 0)
     {
-      /* Read the next x and y positions. */
+      /* Read the next x and y positions from the FIFO. */
 
 #ifdef CONFIG_STMPE11_SWAPXY
       x = stmpe11_getreg16(priv, STMPE11_TSC_DATAX);
@@ -972,6 +958,25 @@ void stmpe11_tscworker(FAR struct stmpe11_dev_s *priv, uint8_t intsta)
       y = stmpe11_getreg16(priv, STMPE11_TSC_DATAX);
 #endif
 
+      /* If we have not yet processed the last pen up event, then we
+       * cannot handle this pen down event. We will have to discard it.  That
+       * should be okay because there will be another FIFO event right behind
+       * this one.  Other kinds of data overruns are not harmful.
+       *
+       * Hmm.. a better design might be to disable FIFO interrupts when we
+       * detect pen up.  Then re-enable them when CONTACT_UP is reported.
+       * That would save processing interrupts just to discard the data.
+       */
+
+      if (priv->sample.contact == CONTACT_UP)
+        {
+          /* We have not closed the loop on the last touch ... don't report
+           * anything.
+           */
+
+          goto ignored;
+        }
+
       /* Perform a thresholding operation so that the results will be more stable */
 
       xdiff = x > priv->threshx ? (x - priv->threshx) : (priv->threshx - x);
@@ -983,7 +988,8 @@ void stmpe11_tscworker(FAR struct stmpe11_dev_s *priv, uint8_t intsta)
 
       if (xdiff + ydiff < 6)
         {
-          /* Little or no change in position... don't report */
+          /* Little or no change in position ... don't report anything.
+           */
 
           goto ignored;
         }

nuttx/include/nuttx/wqueue.h

@@ -1,8 +1,8 @@
 /****************************************************************************
  * include/nuttx/wqueue.h
  *
- *   Copyright (C) 2009, 2011 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2009, 2011-2012 Gregory Nutt. All rights reserved.
- *   Author: Gregory Nutt <spudmonkey@racsa.co.cr>
+ *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -160,6 +160,22 @@ EXTERN int work_cancel(struct work_s *work);
 
 #define work_signal() kill(g_worker, SIGWORK)
 
+/****************************************************************************
+ * Name: work_available
+ *
+ * Description:
+ *   Check if the work structure is available.
+ *
+ * Input parameters:
+ *   None
+ *
+ * Returned Value:
+ *   true if available; false if busy (i.e., there is still pending work).
+ *
+ ****************************************************************************/
+
+#define work_available(work) ((work)->worker == NULL)
+
 #undef EXTERN
 #ifdef __cplusplus
 }