Skip to content

touch button update, fix display settings font and size#6360

Merged
arendst merged 1 commit intoarendst:developmentfrom
gemu2015:display-update
Sep 6, 2019
Merged

touch button update, fix display settings font and size#6360
arendst merged 1 commit intoarendst:developmentfrom
gemu2015:display-update

Conversation

@gemu2015
Copy link
Contributor

@gemu2015 gemu2015 commented Sep 6, 2019

Description:

Related issue (if applicable): fixes #<#6357>

displayfont, displaysize now immediately set font and size
touch button interface rework.

remark:
on pre core 2.6 the touch i2c driver is unreliable. until now i could not figure out what is wrong
obviously there must be changes in the i2c driver.

Checklist:

  • The pull request is done against the latest dev branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR.
  • The code change is tested and works on core 2.3.0, 2.4.2 and 2.5.2
  • The code change pass travis tests. Your PR cannot be merged unless tests pass
  • I accept the CLA.

@arendst arendst merged commit 1d4fac0 into arendst:development Sep 6, 2019
@gemu2015 gemu2015 deleted the display-update branch September 6, 2019 10:19
@ascillato
Copy link
Contributor

ascillato commented Sep 6, 2019

@gemu2015

on pre core 2.6 the touch i2c driver is unreliable. until now i could not figure out what is wrong
obviously there must be changes in the i2c driver.

Please, can you test if with the actual arduino STAGE core from today this I2C issue is still there?
Thanks.

@Jason2866
Copy link
Collaborator

Jason2866 commented Sep 6, 2019

@gemu2015 in my build core pre2.6 i use a different i2c (wip) driver which saves about 600 bytes of iram. PR esp8266/Arduino#6326
This driver works fine for the standard Tasmota builds, but it seems it does make problems with the touch driver...
So as @ascillato suggested try Arduino stage core version. They still use the old i2c driver.
But you have to disable features using iram...

@ascillato
Copy link
Contributor

But you have to disable features using iram...

Yes, that is right. I could compile STAGE by disabling:

  • IR Reception/Send
  • Tuya Support
  • Emulation Support
  • mDNS Support

@gemu2015
Copy link
Contributor Author

gemu2015 commented Sep 6, 2019

as it turns out it is not related to pre 2.6
the bug also appears in 2.52.
it is extremely strange as i can not see any reason why this happens.

this routine returns wrong values on 2.52 and above
and works well in 2.42
(it falsely returns no zero values when there is actually no hit)

has anybody a hint what might be wrong ?

original code called with reg 2 (number of touch points)

uint8_t FT6236readTouchRegister(uint8_t reg)
{
Wire.beginTransmission(FT6236_i2c_addr);
Wire.write(reg); // register 0
uint8_t retVal = Wire.endTransmission();
uint8_t returned = Wire.requestFrom(FT6236_i2c_addr,uint8_t(1));
if (Wire.available())
{
retVal = Wire.read();
}
return retVal;
}

@gemu2015
Copy link
Contributor Author

gemu2015 commented Sep 9, 2019

i checked the i2c changes between 2.42 and 2.52 but couldn't figure out what could be the reason for the touch controller failure.

a simple workaround fixed the problem however => reading the status register twice

while the 1. reading delivers wrong readings every few seconds, the 2.reading is always correct

Strange!

@Jason2866
Copy link
Collaborator

Jason2866 commented Sep 9, 2019

Could this be a clock strech issue? There is still a open issue to.

@joba-1
Copy link
Contributor

joba-1 commented Sep 9, 2019

There are a few things wrong with FT6236readTouchRegister().

  1. return code does not indicate failure (have you read button touched/not touched or error?)
  2. return value of endTransmission not used (cannot be sure you now really read reg with requestFrom() or some other register)
  3. return code from requestFrom() is not checked (might be ok since available() probably implies returncode was ok.
  4. available() seems redundant, since requestFrom() just told you in returned, if there is a byte available or not.

Basically, if all works ok, you get the correct button touch state, but in case of error you cannot detect it. I2C is a shared bus, meaning things will go wrong at times.

How about sth like this?:

https://github.com/joba-1/HumiControl/blob/master/src/main.cpp#L162

With that you could see what the "real" error is. Maybe a timing issue. Maybe bus congestion, Maybe the datasheet demands an unusual delay somewhere, Whatever...

HTH

@gemu2015
Copy link
Contributor Author

gemu2015 commented Sep 9, 2019

@Jason2866
the touch controller doesn't use clock stretching.

@joba-1
all you arguments are coherent but
core 2.3 and 2.42 NEVER show an error in this application
while above core 2.42 there are frequent errors every few seconds.
and that with 2 different hardware circuits and 2 different (but compatible) touch controllers
from Focaltech.

@joba-1
Copy link
Contributor

joba-1 commented Sep 9, 2019

not saying my proposals will fix the error, just that modifying the code to check and report errors instead of guessing could help finding the actual cause :)

@gemu2015
Copy link
Contributor Author

gemu2015 commented Sep 9, 2019

you are right!

Wire.beginTransmission(FT6236_i2c_addr);
Wire.write(reg); // register 2
uint8_t retVal = Wire.endTransmission();

Wire.endTransmission returns error 3 (received NACK on transmit of data)

but what difference in the core causes this ???
the second call never fails.

since the workaround fixes the problem i will not further investigate

but this may affect other i2c devices also

@joba-1
Copy link
Contributor

joba-1 commented Sep 9, 2019

ok, maybe the error has nothing to do with this call.

It could well be, the previous i2c access (from what ever) leaves the bus in an inconsistent state and the first stop at the end of endTransmission() sorts this out.

@joba-1
Copy link
Contributor

joba-1 commented Sep 9, 2019

me again, lol. Interesting how much can happen in so few code lines.

What about the default stop parameter of endTransmission(bool stop = true)?
Default is to release the bus which usually is not what you want if you set a register address intending to read from it. I would set stop to false here.
Ok, I'm sure some i2c devices out there might actually need a stop=true, but maybe here it is the other way round and with the new core a false is now not only better, but required.

Or maybe it is just another core bug...

Whatever, I don't have the device and can't test, so I guess I'm out of here now, too.

@gemu2015
Copy link
Contributor Author

ok, i was curious what is going on and took my logic analyzer.
strange extra clocks in cores above 2.42 !!!
what was changed between 2.42 and 2.52 was the twi slave interface added. probably this interferes here? in the twi master i could not figure out a difference that would cause this.
(i have only one device on the i2c bus)
someone with a better knowledge of i2c secrets should look at this problem.

core pre2.6
core_pre2 6
core 2.42
core_2 42

@gemu2015
Copy link
Contributor Author

found the bug!
as i expected the slave mode has a bug. it enables CHANGE interrupts on scl and sda.
commenting this out solves the problem.

i don't know however why this doesn't affect other sensors.

very strange

void Twi::init(unsigned char sda, unsigned char scl)
{
// set timer function
ets_timer_setfn(&timer, onTimer, NULL);

// create event task
ets_task(eventTask, EVENTTASK_QUEUE_PRIO, eventTaskQueue, EVENTTASK_QUEUE_SIZE);

twi_sda = sda;
twi_scl = scl;
pinMode(twi_sda, INPUT_PULLUP);
pinMode(twi_scl, INPUT_PULLUP);
twi_setClock(preferred_si2c_clock);
twi_setClockStretchLimit(230); // default value is 230 uS

if (twi_addr != 0)
{
 //   attachInterrupt(scl, onSclChange, CHANGE);
 //   attachInterrupt(sda, onSdaChange, CHANGE);
}

}

@Jason2866
Copy link
Collaborator

Have you addressed your finding at Arduino ESP8266 github?
Would help ;-)

@schliepi
Copy link

Sure, removing the attachInterrupt will solve your slave mode problem as it disables slave mode. Simply call Wire.begin() without address when you do not want slave mode. A master does not need any address because it will never send the own address to anyone. Neither does anyone need the address of the master, because it cannot even get addressed, because you disabled slave mode.

There may be bugs in slave mode, @Jason2866 cross referenced 5762. But this is due to esp8266 being too slow for handling interrupts for i2c running at 100kHz.

@gemu2015
Copy link
Contributor Author

yes thats the difference. in the touch controller code i got from the net there was a wire.begin(address) a simple wire.begin() solves the problem.

thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants