[bug fix and improvement] LiquidCrystal Library - Improve timing to make LESS blocking #4550

Closed
wants to merge 6 commits into
from

Conversation

5 participants
@techpaul
Contributor

techpaul commented Feb 9, 2016

Tested using V1.6.7 IDE and DUE R3 hardware, 20 x 4 LCD in 4 bit and 8 bit mode

Current pulseEnable function always blocks AFTER sending data for 100 us.

Revision notes time (us) and delays amount needed to either get to the required delay or minimum time for how long Enable pin must be high. This way shorter delays inside pulseEnable take account of other software time processing. shortening any delay time.

Created a define at top of cpp file for minimu time between writes, and reduced to 80 us, probably could go lower but at just over twice the chip datasheet timing required, will allow for slow clones of the chip.

Test code in attached zip does a setCursor and write 20 characters to a line, measured time improvements of just under 1ms.This equates to a time saving of 20% for 4 bit mode and 33% for 8 bit mode. There are plenty of scope screenshots of the signals before and after for 4 and 8 bit modes also in the zip file. Example after change where 2.67 ms print became 1.70 ms -
new-8bit-20chars

This method actually means you can scope the Enable pin and see how long it is in delay (high) and how long is rest of software (low)
testdata.zip

The major part of the time saving comes from code changes to work out how long to delay.

@techpaul

This comment has been minimized.

Show comment
Hide comment
@techpaul

techpaul Feb 10, 2016

Contributor

Does anyone know any displays that have a LONG write delay time ?

Currently Pull request sets the write delay time to 80 us, where as reducing this to 40 us (still more than the HD44780 base spec of 37 us) can mean a set cursor and 20 character write can be reduced to 900 us. I personally will use for myself 40 us write delay but wish to be sure no others are affected if a further release change was made to 40 us.

Any thoughts welcome

Contributor

techpaul commented Feb 10, 2016

Does anyone know any displays that have a LONG write delay time ?

Currently Pull request sets the write delay time to 80 us, where as reducing this to 40 us (still more than the HD44780 base spec of 37 us) can mean a set cursor and 20 character write can be reduced to 900 us. I personally will use for myself 40 us write delay but wish to be sure no others are affected if a further release change was made to 40 us.

Any thoughts welcome

@timkay

This comment has been minimized.

Show comment
Hide comment
@timkay

timkay Feb 10, 2016

According to the docs, if the wiring is right (R/W, pin 5), it's possible to poll the LCD controller to see if it is ready for the next write. With this approach, there is no guesswork; the data would be written at exactly the right rate for any display.

timkay commented Feb 10, 2016

According to the docs, if the wiring is right (R/W, pin 5), it's possible to poll the LCD controller to see if it is ready for the next write. With this approach, there is no guesswork; the data would be written at exactly the right rate for any display.

@techpaul

This comment has been minimized.

Show comment
Hide comment
@techpaul

techpaul Feb 10, 2016

Contributor

That is just as blocking if spin lock on that,, also depends if someone has wired up the R/W pin which library allows you to do or not do. So time delay is only way for ALL wiring configurations supported by the library and examples all over Arduino and other related sites.

Hopefully there does not exist a display where R/W goes nowhere (but I would not be surprised if one existed)

Contributor

techpaul commented Feb 10, 2016

That is just as blocking if spin lock on that,, also depends if someone has wired up the R/W pin which library allows you to do or not do. So time delay is only way for ALL wiring configurations supported by the library and examples all over Arduino and other related sites.

Hopefully there does not exist a display where R/W goes nowhere (but I would not be surprised if one existed)

@timkay

This comment has been minimized.

Show comment
Hide comment
@timkay

timkay Feb 10, 2016

It's not a matter of R/W going nowhere... It's a question whether the design includes access to R/W. Some designs simply hard wire R/W, so that it is write-only. It would be interesting to have an option in the library that lets one provide the arduino pin number of the R/W pin. The software could then test to see if it works, and if it does, delay could be minimized.

timkay commented Feb 10, 2016

It's not a matter of R/W going nowhere... It's a question whether the design includes access to R/W. Some designs simply hard wire R/W, so that it is write-only. It would be interesting to have an option in the library that lets one provide the arduino pin number of the R/W pin. The software could then test to see if it works, and if it does, delay could be minimized.

@techpaul

This comment has been minimized.

Show comment
Hide comment
@techpaul

techpaul Feb 10, 2016

Contributor

I suggest you look at Liquid Crystal Libray in
particular the class constructor definition which has R/W or not R/W pin overloaded types. Adding looking using R/W pin if defined would increase the code size for little benefit. As many designs and examples hard wire it to GND

Contributor

techpaul commented Feb 10, 2016

I suggest you look at Liquid Crystal Libray in
particular the class constructor definition which has R/W or not R/W pin overloaded types. Adding looking using R/W pin if defined would increase the code size for little benefit. As many designs and examples hard wire it to GND

@techpaul

This comment has been minimized.

Show comment
Hide comment
@techpaul

techpaul Feb 11, 2016

Contributor

Did some further checking on the datasheet for HD44780 Busy flag checking is only valid after the initail function set command (controller knows if 4 or 8 bit mode) this means extra checks would be needed to delay for a time if this has not been done yet. Additionally it most people follow Arduino's example of forcing R/W pin to GND (as all Arduino, Adafruit and other starter examples show) it would be of little gain.

HOWEVER I have noticed something from the datasheet that could mean 4 bit interfacing could also be speeded up more to get closer to 8 bit interface timing. Needs testing.

Contributor

techpaul commented Feb 11, 2016

Did some further checking on the datasheet for HD44780 Busy flag checking is only valid after the initail function set command (controller knows if 4 or 8 bit mode) this means extra checks would be needed to delay for a time if this has not been done yet. Additionally it most people follow Arduino's example of forcing R/W pin to GND (as all Arduino, Adafruit and other starter examples show) it would be of little gain.

HOWEVER I have noticed something from the datasheet that could mean 4 bit interfacing could also be speeded up more to get closer to 8 bit interface timing. Needs testing.

@techpaul

This comment has been minimized.

Show comment
Hide comment
@techpaul

techpaul Feb 11, 2016

Contributor

RIght I have done 4 bit mods and now using same test method the 4 bit mode timings have reduced DRAMATICALLY !

Test send setcursor and 20 character string to LCD

Original test 4.34 ms
Optimal result 1.23 ms

Yes that is 3.11ms faster, yes nearly a quarter of the time in old library.

Because problem in 4 bit mode is the second nibble can be written 1 us later, but library was using the dealy for between COMPLETE commands between each nibble.

Comparison scope shots and a text file in updated
testdata.zip

Update to github shortly.

Contributor

techpaul commented Feb 11, 2016

RIght I have done 4 bit mods and now using same test method the 4 bit mode timings have reduced DRAMATICALLY !

Test send setcursor and 20 character string to LCD

Original test 4.34 ms
Optimal result 1.23 ms

Yes that is 3.11ms faster, yes nearly a quarter of the time in old library.

Because problem in 4 bit mode is the second nibble can be written 1 us later, but library was using the dealy for between COMPLETE commands between each nibble.

Comparison scope shots and a text file in updated
testdata.zip

Update to github shortly.

techpaul added some commits Feb 11, 2016

LCD Library 4 bit improvements
Reduce delay between nibbles as per Datasheet
Long delays are for between COMPLETE commands NOT nibbles of a byte

Tidy functions write8bits and write4bits become one function and
pass in bit size. Original functions were copy/paste with one number
change between them.

Change constant define from _MIN_WRITE_DELAY
to more meaningful   _LCD_COMMAND_DELAY

@techpaul techpaul changed the title from LiquidCrystal Library - Improve timing to make LESS blocking to [bug fix and improvement] LiquidCrystal Library - Improve timing to make LESS blocking Feb 11, 2016

@techpaul

This comment has been minimized.

Show comment
Hide comment
@techpaul

techpaul Feb 11, 2016

Contributor

Finalised this now

Bug fixes

  1. Change 4 bit mode nibble to wait 1us between nibbles and the command delay time between full 8 bit command values
  2. Change delays between command to work out how much longer to wait instead of always blocking after each nibble or byte for 100us extra than any code overhead.

Improvements

1/ Reduce command delay time closer to chip spec of 37us (use 40us as default)
2/ add method to be called before begin to allow longer setting of command delay up to 200 us
(doubt any will need it something that would be better served with a sketch custom.h include for library)
3/ Convert two copy pasted functions into one function with extra parameter

Overall performance improvements

on Due Old library set cursor and write 20 chars

4 bit mode 4.34 ms
8 bit mode 2.67 ms

Latest version
4 bit mode 1.23 ms
8 bit mode 0.9 ms

On Mega latest version only timings
4 bit mode 2.57 ms
8 bit mode 2.13 ms

Contributor

techpaul commented Feb 11, 2016

Finalised this now

Bug fixes

  1. Change 4 bit mode nibble to wait 1us between nibbles and the command delay time between full 8 bit command values
  2. Change delays between command to work out how much longer to wait instead of always blocking after each nibble or byte for 100us extra than any code overhead.

Improvements

1/ Reduce command delay time closer to chip spec of 37us (use 40us as default)
2/ add method to be called before begin to allow longer setting of command delay up to 200 us
(doubt any will need it something that would be better served with a sketch custom.h include for library)
3/ Convert two copy pasted functions into one function with extra parameter

Overall performance improvements

on Due Old library set cursor and write 20 chars

4 bit mode 4.34 ms
8 bit mode 2.67 ms

Latest version
4 bit mode 1.23 ms
8 bit mode 0.9 ms

On Mega latest version only timings
4 bit mode 2.57 ms
8 bit mode 2.13 ms

@techpaul

This comment has been minimized.

Show comment
Hide comment
@techpaul

techpaul Mar 8, 2016

Contributor

Any chance someone actually looking at this as I see the existing library had its version updated to V1.0.5 by @facchinm

Contributor

techpaul commented Mar 8, 2016

Any chance someone actually looking at this as I see the existing library had its version updated to V1.0.5 by @facchinm

@bperrybap

This comment has been minimized.

Show comment
Hide comment
@bperrybap

bperrybap Aug 21, 2016

It has been clear to me for many years that whoever wrote the begin() and pulseEnable() code did not understand how the hd44780 interface really works.

in pulseEnable(), things like the comment about commands needing time "to settle" and delaying the 100us between nibbles made no sense.

This patch update resolves some of what was wrong with pulseEnable(); and the blind command delays by handling the LCD execution times in a smarter way however, if a new API function is going to be added it should handle configuring all the necessary execution times to make sure that it really allows slower LCDs to fully work.
This patch will not do that. So while it is interesting, it will not allow the library to be used with slower LCDs and I would encourage not adopting a new API function to configure slower execution times that doesn't serve its intended purpose - which is to allow slower LCDs to work with the library.

To support LCDs that use a clock slower than the 270 kHz reference clock, the code must handle being able to configure not only the command/data instruction times but also the clear/home execution times.
This patch only handles the command/data execution times.
When the LCD is slower than the 270 kHz reference clock frequency, the times in table 6 of the hd44780 spec are longer and so clear/home will also be longer and need adjustment as well.

So to really work with slower LCDs, extending and configuring both execution times must be supported.

The LCD API 1.0 spec
http://playground.arduino.cc/Code/LCDAPI defined the function setDelay(cmd,char) to handle configuring multiple delays. Their description of this is a bit mushy.
What is really needed is an API function to set clear/home and cmd/data execution times.
The setDelay(cmd,char) could be morphed into serving that purpose if you use the cmd parameter to configure the clear/home execution time and the char to configure the data/instruction times.
Keep in mind that the clear/home times are much longer than 255us so the argument type at least for that parameter has to be larger than uint8_t

I have a hd44780 library that implements proper execution timing and supports the configuration of the two different execution times through a function called setExecTimes(chUs, insUs)
It is located here: https://github.com/duinoWitchery/hd44780
The hd44780 library code is GPL 3.0 so none of it can be used, moved, or replicated into the LiquidCrystal library unless the the LiquidCrystal library is moved to GPL 3.0

If adding a new API call to LiquidCrystal, my suggestion would be to first fix the code to support both types of execution times and then add a new API function that allows setting both the clear/home and cmd/data executions and call it either:
setDelay(cmd,char) or preferably setExecTimes(chUS, insUs)

The hd44780 library comes with a sketch that will time the interface and give you a byte transfer time. This is a great tool for comparing i/o speed.
It is down in examples/hd44780examples/LCDiSpeed
You can use this to time and compare byte transfer times for various libraries.
It would be interesting to compare the 4bit mode transfer times of your patched library with the original LiquidCrystal vs the hd44780 pinIO class.

--- bill

It has been clear to me for many years that whoever wrote the begin() and pulseEnable() code did not understand how the hd44780 interface really works.

in pulseEnable(), things like the comment about commands needing time "to settle" and delaying the 100us between nibbles made no sense.

This patch update resolves some of what was wrong with pulseEnable(); and the blind command delays by handling the LCD execution times in a smarter way however, if a new API function is going to be added it should handle configuring all the necessary execution times to make sure that it really allows slower LCDs to fully work.
This patch will not do that. So while it is interesting, it will not allow the library to be used with slower LCDs and I would encourage not adopting a new API function to configure slower execution times that doesn't serve its intended purpose - which is to allow slower LCDs to work with the library.

To support LCDs that use a clock slower than the 270 kHz reference clock, the code must handle being able to configure not only the command/data instruction times but also the clear/home execution times.
This patch only handles the command/data execution times.
When the LCD is slower than the 270 kHz reference clock frequency, the times in table 6 of the hd44780 spec are longer and so clear/home will also be longer and need adjustment as well.

So to really work with slower LCDs, extending and configuring both execution times must be supported.

The LCD API 1.0 spec
http://playground.arduino.cc/Code/LCDAPI defined the function setDelay(cmd,char) to handle configuring multiple delays. Their description of this is a bit mushy.
What is really needed is an API function to set clear/home and cmd/data execution times.
The setDelay(cmd,char) could be morphed into serving that purpose if you use the cmd parameter to configure the clear/home execution time and the char to configure the data/instruction times.
Keep in mind that the clear/home times are much longer than 255us so the argument type at least for that parameter has to be larger than uint8_t

I have a hd44780 library that implements proper execution timing and supports the configuration of the two different execution times through a function called setExecTimes(chUs, insUs)
It is located here: https://github.com/duinoWitchery/hd44780
The hd44780 library code is GPL 3.0 so none of it can be used, moved, or replicated into the LiquidCrystal library unless the the LiquidCrystal library is moved to GPL 3.0

If adding a new API call to LiquidCrystal, my suggestion would be to first fix the code to support both types of execution times and then add a new API function that allows setting both the clear/home and cmd/data executions and call it either:
setDelay(cmd,char) or preferably setExecTimes(chUS, insUs)

The hd44780 library comes with a sketch that will time the interface and give you a byte transfer time. This is a great tool for comparing i/o speed.
It is down in examples/hd44780examples/LCDiSpeed
You can use this to time and compare byte transfer times for various libraries.
It would be interesting to compare the 4bit mode transfer times of your patched library with the original LiquidCrystal vs the hd44780 pinIO class.

--- bill

@techpaul

This comment has been minimized.

Show comment
Hide comment
@techpaul

techpaul Aug 21, 2016

Contributor

In my personal opinion a lot of the Arduino codespace is polluted with bad practises and bad code, too many uses of delays in ms range, bad structuring and 'oh this is good because it worked for me'. Too much forcing of very low level stuff into classes and the overhead of that as shown with the begin() method.

Don't even get me on the lack of common error handling, let alone libraries that have comments like
"Must find a way of reporting this error to highre level".

No real structure to the libraries and no in built scheduling, amongst other things like, everything is linear code is a cul-de-sac environment. Too many ludges in my view on the whole software environment, as everything has to be forced to be like AVR.

To that end all my Arduino work has now been moved to LEGACY code as too many folks are interested only in the Legacy device of the Uno as being the only platform.

Contributor

techpaul commented Aug 21, 2016

In my personal opinion a lot of the Arduino codespace is polluted with bad practises and bad code, too many uses of delays in ms range, bad structuring and 'oh this is good because it worked for me'. Too much forcing of very low level stuff into classes and the overhead of that as shown with the begin() method.

Don't even get me on the lack of common error handling, let alone libraries that have comments like
"Must find a way of reporting this error to highre level".

No real structure to the libraries and no in built scheduling, amongst other things like, everything is linear code is a cul-de-sac environment. Too many ludges in my view on the whole software environment, as everything has to be forced to be like AVR.

To that end all my Arduino work has now been moved to LEGACY code as too many folks are interested only in the Legacy device of the Uno as being the only platform.

@facchinm facchinm referenced this pull request in arduino-libraries/LiquidCrystal Jul 25, 2017

Open

Improve timing to make LESS blocking #14

@facchinm

This comment has been minimized.

Show comment
Hide comment

@facchinm facchinm closed this Jul 25, 2017

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