From 4ddf48475760e7d5b22e391573a940fd604847f8 Mon Sep 17 00:00:00 2001 From: patacongo Date: Fri, 11 May 2012 22:07:06 +0000 Subject: Fix a few STMPE11 touchscreen and NxWM touchscreen calibration bugs git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@4723 42af7a65-404d-4744-a932-0658087f49c3 --- NxWidgets/ChangeLog.txt | 13 ++++--- NxWidgets/UnitTests/nxwm/main.cxx | 32 +++++++++++------ NxWidgets/nxwm/include/ccalibration.hxx | 1 + NxWidgets/nxwm/include/ctouchscreen.hxx | 12 +------ NxWidgets/nxwm/include/nxwmconfig.hxx | 9 ++++- NxWidgets/nxwm/src/ccalibration.cxx | 43 +++++++++++++++++------ NxWidgets/nxwm/src/ctouchscreen.cxx | 54 +++++++++++++++++++++++++++- nuttx/ChangeLog | 7 +++- nuttx/drivers/input/stmpe11_base.c | 2 +- nuttx/drivers/input/stmpe11_tsc.c | 62 ++++++++++++++++++--------------- nuttx/include/nuttx/wqueue.h | 20 +++++++++-- 11 files changed, 186 insertions(+), 69 deletions(-) diff --git a/NxWidgets/ChangeLog.txt b/NxWidgets/ChangeLog.txt index 1367c8f68..38e902242 100755 --- a/NxWidgets/ChangeLog.txt +++ b/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 - window applications. +* NxWM::CFullScreenWindow and NxWM::CTaskbar: Add support in NxWM for full + 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 - to go). +* NxWM::CTouchscreen and NxWM::CCalibration: Add touchscreen support (still a long + 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. diff --git a/NxWidgets/UnitTests/nxwm/main.cxx b/NxWidgets/UnitTests/nxwm/main.cxx index 291267eb8..c7433f36e 100644 --- a/NxWidgets/UnitTests/nxwm/main.cxx +++ b/NxWidgets/UnitTests/nxwm/main.cxx @@ -79,16 +79,16 @@ struct SNxWmTest { - NxWM::CTaskbar *taskbar; // The task bar - NxWM::CStartWindow *startwindow; // The start window + NxWM::CTaskbar *taskbar; // The task bar + NxWM::CStartWindow *startwindow; // The start window #ifdef CONFIG_NXWM_TOUCHSCREEN - NxWM::CTouchscreen *touchscreen; // The touchscreen - NxWM::CCalibration *calibration; // The touchscreen calibration application - struct NxWM::SCalibrationData data; // Calibration data + NxWM::CTouchscreen *touchscreen; // The touchscreen + NxWM::CCalibration *calibration; // The touchscreen calibration application + struct NxWM::SCalibrationData calibData; // Calibration data #endif - unsigned int mmInitial; // Initial memory usage - unsigned int mmStep; // Memory Usage at beginning of test step - unsigned int mmSubStep; // Memory Usage at beginning of test sub-step + unsigned int mmInitial; // Initial memory usage + unsigned int mmStep; // Memory Usage at beginning of test 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 diff --git a/NxWidgets/nxwm/include/ccalibration.hxx b/NxWidgets/nxwm/include/ccalibration.hxx index 42eb56ad2..535a14804 100644 --- a/NxWidgets/nxwm/include/ccalibration.hxx +++ b/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]; diff --git a/NxWidgets/nxwm/include/ctouchscreen.hxx b/NxWidgets/nxwm/include/ctouchscreen.hxx index c3aa198cb..52433c1de 100644 --- a/NxWidgets/nxwm/include/ctouchscreen.hxx +++ b/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) - { - // 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; - } + void setCalibrationData(struct SCalibrationData &caldata); /** * Capture raw driver data. This method will capture mode one raw touchscreen diff --git a/NxWidgets/nxwm/include/nxwmconfig.hxx b/NxWidgets/nxwm/include/nxwmconfig.hxx index 4e51cea1d..359f7d095 100644 --- a/NxWidgets/nxwm/include/nxwmconfig.hxx +++ b/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 diff --git a/NxWidgets/nxwm/src/ccalibration.cxx b/NxWidgets/nxwm/src/ccalibration.cxx index 9ba0c0917..4c6a29ed9 100644 --- a/NxWidgets/nxwm/src/ccalibration.cxx +++ b/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", - m_state, m_screenInfo.pos.x, m_screenInfo.pos.y, - m_touchPos.x, m_touchPos.y); + if (sample.point[0].id == m_touchId) + { + // 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; } diff --git a/NxWidgets/nxwm/src/ctouchscreen.cxx b/NxWidgets/nxwm/src/ctouchscreen.cxx index f5e476cce..21ccc91e3 100644 --- a/NxWidgets/nxwm/src/ctouchscreen.cxx +++ b/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; } diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog index b33ad446f..4b5ab66d0 100644 --- a/nuttx/ChangeLog +++ b/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. diff --git a/nuttx/drivers/input/stmpe11_base.c b/nuttx/drivers/input/stmpe11_base.c index 34d13aeac..546f1d901 100644 --- a/nuttx/drivers/input/stmpe11_base.c +++ b/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) { diff --git a/nuttx/drivers/input/stmpe11_tsc.c b/nuttx/drivers/input/stmpe11_tsc.c index 09b06d497..c5aae3830 100644 --- a/nuttx/drivers/input/stmpe11_tsc.c +++ b/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 - * 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? */ + /* Is there new STMPE11 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 - * of semphores from interrupt handlers, and (2) to prevent sampled data - * 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. + /* Disable pre-emption to prevent the worker thread from running + * asynchronously. */ 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; } diff --git a/nuttx/include/nuttx/wqueue.h b/nuttx/include/nuttx/wqueue.h index 5f5fecef8..dfd424c8d 100644 --- a/nuttx/include/nuttx/wqueue.h +++ b/nuttx/include/nuttx/wqueue.h @@ -1,8 +1,8 @@ /**************************************************************************** * include/nuttx/wqueue.h * - * Copyright (C) 2009, 2011 Gregory Nutt. All rights reserved. - * Author: Gregory Nutt + * Copyright (C) 2009, 2011-2012 Gregory Nutt. All rights reserved. + * Author: Gregory Nutt * * 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 } -- cgit v1.2.3