Rockbox

The patch itself.

   touchscreen_cowond2.patch (6.8 KiB)

Sorry, a typo in the old patch.

   touchscreen_cowond2.patch (6.8 KiB)

Thanks for this. There are few things that need to be resolved before this can go in the main build:

- you can get the PMU type by calling get_pmu_type() defined in power-target.h
- all the touchscreen code should probably be moved out of button-cowond2.c into a separate file, since it’s turning in to a bit of a monster now
- consider making button_read_touch_* always return 10-bit precision (as per the PCF), this would solve your ‘short’ issues and not really cause any reduction in accuracy
- it’s probably a waste to poll the touchscreen every 100Hz, maybe cache the results and poll every 10 ticks or so?

Also I was thinking of removing the touch_calibration stuff since there’s already a calibration screen in the settings menu (which could be set up with sensible defaults). That would remove some of the complexity.

I agree with all your comments, and I’ll update my patch. I’ll look into the calibration code as well.

Hi Rob, I hope this patch is to your satisfaction.

   touchscreen_cowond2_ver2.patch (14.8 KiB)

That’s looking pretty good, although if possible I’d like to try and tidy up the ‘s before it gets committed (although the I2C related names are going to be tricky if we can’t identify the device). The reason is simple: most probably no-one will ever get around to fixing them…

A few minor comments:

- it’d be more consistent to use the TIME_AFTER macro to check whether to read the touchscreen or not, rather than a simple counter (see example in touchscreen_read_pcf50606)
- the patch contains a mixture of TABs and spaces, and we don’t like TABs - the calibration constants you added turn out to be fairly similar to the existing ones (which aren’t accurate anyway), maybe the existing ones or a ‘halfway house’ would work for you?

Apart from the last one, these are things I can fix myself before committing. If the old calibration values work for you (it doesn’t need to be accurate - that’s what the config option is for…) I will commit this later today. The extra calibration step will probably be removed entirely anyway, as it is a relic from before the config option existed.

Finally one more thing, how good is the data coming from this ‘unknown’ device? Does it need to be smoothed at all like the data from the PCF50606? You can check quite easily by adding the test_touchscreen plugin to plugins/SOURCES and running it see how much the pointer jitters…..

Feel free to do what changes you feel needed. Is there any code-style checking patch, like checkpatch for the linux kernel?
Ah, forgot about the TIME_AFTER macro - of course it should be used.
The old pcf50606 calibration values are nearly the same as mine. Yes, your old ones works fine.

I tested with the test_touchscreen plugin and the pointer isn’t steady - or rather I guess my hand isn’t. It tend to flicker on a I guess one pixel level, mostly in X direction.

I’m pretty confident that the unknown touchscreen device is an TI TSC2003 - or something very alike. I have not gotten the “Measure” parts of the commands to work yet, but the others seems to respond properly.

If you’re only getting jitter of 1px or so then the accuracy is already far better than the PCF50606 equivalent. I would guess the touchscreen controller hardware is doing some smoothing of the data - which would probably make it a TSC2007:

“If the acquired signal source is noisy because of the digital switching circuit, it may necessary to evaluate the
data without noise. In this case, the median value filter operation helps remove the noise. The array of seven
converted results is sorted first. The middle three values are then averaged to produce the output value of the
MAV filter.
The MAV filter is applied to all measurements for all analog inputs including the touch screen inputs, temperature
measurements TEMP1 and TEMP2, and auxiliary input AUX.”

(from http://focus.ti.com/lit/ds/symlink/tsc2007.pdf )

Here’s the update I intend to commit, could you check it still works on your D2+ as I have moved a few things around!

ps. to answer your earlier question, we don’t have anything as fancy as a patch checking script - just a few basic guidelines in docs/CONTRIBUTING.

EDIT: updated patch to include a missing file

   touchscreen_cowond2_ver3.patch (18.8 KiB)

You are probably right, it is an TSC2007. Sounds more like it.
Your patch looks fine, but I tested your patch and it didn’t work. I haven’t had the time to find out what it wrong with it. I’ll do that tomorrow.
Ah, the documentation. Sorry forgot about it since you have been so helpful. Thanks!

Strange, I wonder what the problem is.

I thought it might be because we now check the return values from i2c_readmem() - but our implementation (in i2c-telechips.c) always seems to return 0, so it can’t be that. I hope it doesn’t take too much effort to debug my mistake :/

After fixing wwo small errors your patch works:

- Should be TIME_BEFORE instead of TIME_AFTER in touchscreen_read_tsc200x(), touchscreen-cowond2.c
- Add a line saying “last_btn = btn” at the end of touchscreen_read_tsc200x(), touchscreen-cowond2.c

Then it works fine.

Ah yes, I hadn’t noticed the logic was backwards… but regarding ‘last_btn = btn’, the ver2 of this patch didn’t do this - presumably it didn’t work very well either

I’ll commit your changes tonight, many thanks for working on this.