Using LCD causes error in Senor reading

Hello,

I haven't done much programming with Arduino, compared to my previous projects this one is kind of a major project.

I have one of these DIY Filament extruder ,
I am trying to add Filament puller and filament diameter sensor to create a closed loop system which would give a controlled diameter tolerance. Inspired by Meet the Composer and Precision - Desktop Filament Makers | 3devo

I am using a Digital caliper to measure the diameter. I have written code to read the diameter and send it over to Serial if value is different that the previous one, and use this a s Input to PID, compare it with Setpoint and produce output, that will be used to control the speed of DC motor.

I can read the caliper data(Sometimes there're glitches). I decoded to add a 16x2 LCD so that I can view the measured diameter and in later stage add a rotary encode, so that I could change Kp, Ki, Kd and Setpoint values.

But when added code to display the measured diameter on LCD, both Serial and LCD data are incorrect and changes rapidly.
With out LCD code, Serial terminal displays same reading as shown in Caliper display.

Here's the code,

//Digital caliper code to read the value off of a cheap set of digital calipers
//By Making Stuff Youtube channel https://www.youtube.com/c/makingstuff
//This code is open source and in the public domain.

#include <PID_v1.h>

// include the library code:
#include <LiquidCrystal.h>
//#include <LiquidCrystal_I2C.h>

#define PIN_OUTPUT 5    // To motor driver

//Define Variables we'll be connecting to
double Setpoint, Input, Output;
uint8_t SampleTime = 20;

// PID MIN & MAX output values
#define MIN   50
#define MAX   255

//Specify the links and initial tuning parameters
double Kp = 2, Ki = 5, Kd = 1;
PID myPID(&Input, &Output, &Setpoint, Kp, Ki, Kd, REVERSE);

// initialize the library by associating any needed LCD interface pin
// with the arduino pin number it is connected to
const int rs = 6, en = 7, d4 = 8, d5 = 9, d6 = 10, d7 = 11;
LiquidCrystal lcd(rs, en, d4, d5, d6, d7);

const byte clockPin = 2;  //attach to clock pin on calipers
const byte dataPin = 3; //attach to data pin on calipers

//Milliseconds to wait until starting a new value
//This can be a different value depending on which flavor caliper you are using.
const int cycleTime = 32;

unsigned volatile int clockFlag = 0;

long now = 0;
long lastInterrupt = 0;
long value = 0;

float finalValue = 0;
float previousValue = 0;

int newValue = 0;
int sign = 1;
int currentBit = 1;

uint8_t previousOutput = 0;

void setup() {
  Serial.begin(115200);

  pinMode(clockPin, INPUT);
  pinMode(dataPin, INPUT);

  // set up the LCD's number of columns and rows:
  lcd.begin(16, 2);

  //We have to take the value on the RISING edge instead of FALLING
  //because it is possible that the first bit will be missed and this
  //causes the value to be off by .01mm.
  attachInterrupt(digitalPinToInterrupt(clockPin), clockISR, RISING);

  //initialize the variables we're linked to
  Setpoint = 1.75;    // Required diameter of filament
  //turn the PID on
  myPID.SetSampleTime(SampleTime);
  myPID.SetOutputLimits(MIN, MAX);
  myPID.SetMode(AUTOMATIC);
}

void loop() {

  if (newValue)
  {
    if (finalValue != previousValue) {
      previousValue = finalValue;
      Serial.println(finalValue, 2);
    }
    newValue = 0;
  }

  //The ISR Can't handle the arduino command millis()
  //because it uses interrupts to count. The ISR will
  //set the clockFlag and the clockFlag will trigger
  //a call the decode routine outside of an ISR.
  if (clockFlag == 1)
  {
    clockFlag = 0;
    decode();
  }

  Input = finalValue;
  myPID.Compute();
  analogWrite(PIN_OUTPUT, Output);

  //lcd.setCursor(0, 1);
  /*
    lcd.print("F:");
    lcd.setCursor(2,1);
    lcd.print(finalValue);
  */
}

void decode() {
  unsigned char dataIn;
  dataIn = digitalRead(dataPin);

  now = millis();

  if ((now - lastInterrupt) > cycleTime)
  {
    finalValue = (value * sign) / 100.00;
    currentBit = 0;
    value = 0;
    sign = 1;
    newValue = 1;
  }
  else if (currentBit < 16 )
  {

    if (dataIn == 0)
    {
      if (currentBit < 16) {
        value |= 1 << currentBit;
      }
      else if (currentBit == 20) {
        sign = -1;
      }

    }

    currentBit++;

  }

  lastInterrupt = now;

}

void clockISR() {
  clockFlag = 1;
}

Even uncommenting

[i]lcd.setCursor(0, 1);[/i]

causes error in reading ?

Why is that?

My doubt is that, delay in LCD functions cause this.

So I thought about using I2C based LCD, but now I think it would be much slower that current one(i2c at 100 KHz).

In attached images,

1.PNG : With out LCD code
2.PNG : with LCD code

Here's some useful links if needed : Reading Digital Calipers This is the code I used. I used BC548 to convert 1.5V from Caliper to 5V

My Caliper data is same as shown in this site(I checked using DSO) : Blog of Wei-Hsiung Huang: Using Digital Caliper For Digital Read Out (DRO) Applications

Also as a doubt, I have seen some people using Stepper motor instead of DC motor, any idea why is that??

Thanks

Arduino_caliper_interrupt_lcd.zip (1.61 KB)

Maybe look at your loop() to see if there are activities there which need not be done every loop iteration. For example, you would be writing to the LCD much more often that the human eye could see. Maybe this should happen only when you print finalValue to the console or when clockFlag has the value of 1.

Do not post ZIP files.

In my caliper, a new data is send every 200 ms, and duration of each packet(24 clock pulse) is around 16 ms. That means each clock pulse is 666 micro seconds.

So If I understand the flow of program correctly, the loop() should not take more than this time to execute the data. I commented out all LCD code and added a delay of 1 micro seconds, program reads correctly. But a delay of 100 micro seconds messes up the reading.

Since the reading might change rapidly there's a high possibility that LCD will have to update a new value.
I have pu the LCD print inside a condition that only update LCD as well as Serial terminal value if the reading is new,

if (newValue)
  {
    if (finalValue != previousValue) {
      previousValue = finalValue;
      Serial.println(finalValue, 2);
      lcd.setCursor(0, 1);
      lcd.print(finalValue);
    }
    newValue = 0;
  }

That stll show two reading on the LCD and terminal alternatively. One is correct another is wrong

OK. So every time you perform a write operation to the LCD, the delay causes the next calculation to fail. Is that it ?

It is true that the LCD libraries may contain blocking code. See for example LCD to use to have the least impact on the execution speed of the arduino - Displays - Arduino Forum post #16

The code your sketch is based on appears to be this: DigitalCalipers/DigitalCalipers.ino at master · MakingStuffChannel/DigitalCalipers · GitHub

It does not have any reference to this library that you are using PID_v1.h. What are ou doing here and does the basic caliper reading code work without the problems you have described ?

The code I'm using is the same as in the github link you have given.

It reads the caliper data and display it on Serial Terminal for every new reading. It works.

What I am trying to do is, Use the Caliper as a Filament diameter sensor, use the reading from it and a PID loop to control the speed of a puller motor which pulls the Extruded filament to control it's diameter.

Here's the video of someone doing it,

AthulSNair:
In my caliper, a new data is send every 200 ms, and duration of each packet(24 clock pulse) is around 16 ms. That means each clock pulse is 666 micro seconds.

So If I understand the flow of program correctly, the loop() should not take more than this time to execute the data.

You understand it correctly. With that type of timing requirement, you will need to do the decoding in the ISR and not try to defer it to loop().
Yeah you might be able to sort of make it work by using a better/faster library like the hd44780 library, but my concern would be that it will likely not work reliably 100% of the time.

I don't see a reason why you can't do the decoding in the ISR.
There is no looping and no dependency on any code that depends on interrupts inside the ISR.
The millis() counter needs interrupts work but does not need interrupts enabled to be called to get the current tick value.

Once you move the decoding to the ISR, then you can use any type of i/o interface on the LCD you want as the ISR can interrupt the LCD code when it needs to.

--- bill

Thanks for the reply.

I was able to make LCD work with above code by making a small modification,

//Digital caliper code to read the value off of a cheap set of digital calipers
//By Making Stuff Youtube channel https://www.youtube.com/c/makingstuff
//This code is open source and in the public domain.
//#include <avr/interrupt.h>
#include <PID_v1.h>

// include the library code:
#include <LiquidCrystal.h>
//#include <LiquidCrystal_I2C.h>


#define PIN_OUTPUT 5    // To motor driver

//Define Variables we'll be connecting to
double Setpoint, Input, Output;
uint8_t SampleTime = 200;

// PID MIN & MAX output values
#define MIN   50
#define MAX   255

//Specify the links and initial tuning parameters
double Kp = 2, Ki = 5, Kd = 1;
PID myPID(&Input, &Output, &Setpoint, Kp, Ki, Kd, REVERSE);

// initialize the library by associating any needed LCD interface pin
// with the arduino pin number it is connected to
const int rs = 6, en = 7, d4 = 8, d5 = 9, d6 = 10, d7 = 11;
LiquidCrystal lcd(rs, en, d4, d5, d6, d7);

const byte clockPin = 2;  //attach to clock pin on calipers
const byte dataPin = 3; //attach to data pin on calipers

//Milliseconds to wait until starting a new value
//This can be a different value depending on which flavor caliper you are using.
const int cycleTime = 100;

unsigned volatile int clockFlag = 0;

long now = 0;
long lastInterrupt = 0;
long value = 0;

float finalValue = 0;
float previousValue = 0;

int newValue = 0;
int sign = 1;
int currentBit = 1;

uint8_t previousOutput = 0;

void setup() {
  Serial.begin(115200);

  pinMode(clockPin, INPUT);
  pinMode(dataPin, INPUT);

  // set up the LCD's number of columns and rows:
  lcd.begin(16, 2);

  //We have to take the value on the RISING edge instead of FALLING
  //because it is possible that the first bit will be missed and this
  //causes the value to be off by .01mm.
  attachInterrupt(digitalPinToInterrupt(clockPin), clockISR, RISING);

  //initialize the variables we're linked to
  Setpoint = 1.75;    // Required diameter of filament
  //turn the PID on
  myPID.SetSampleTime(SampleTime);
  myPID.SetOutputLimits(MIN, MAX);
  myPID.SetMode(AUTOMATIC);
}

void loop() {

  if (newValue)
  {
    if (finalValue != previousValue) {
      previousValue = finalValue;
      Serial.println(finalValue, 2);
    }
    newValue = 0;
  }

  //The ISR Can't handle the arduino command millis()
  //because it uses interrupts to count. The ISR will
  //set the clockFlag and the clockFlag will trigger
  //a call the decode routine outside of an ISR.
  if (clockFlag == 1)
  {
    clockFlag = 0;
    decode();
  }

  Input = finalValue;
  myPID.Compute();
  analogWrite(PIN_OUTPUT, Output);
}

void decode() {
  unsigned char dataIn;
  dataIn = digitalRead(dataPin) + digitalRead(dataPin) + digitalRead(dataPin);

  now = millis();

  if ((now - lastInterrupt) > cycleTime)
  {
    finalValue = (value * sign) / 100.00;
    currentBit = 0;
    value = 0;
    sign = 1;
    newValue = 1;
  }
  else if (currentBit < 16 )
  {

    if (dataIn == 0)
    {
      if (currentBit < 16) {
        value |= 1 << currentBit;
      }
      else if (currentBit == 20) {
        sign = -1;
      }

    }

    currentBit++;

    if (currentBit == 16)
    {
      lcd.setCursor(0, 1);
      lcd.print(finalValue);
      
    }

  }

  lastInterrupt = now;

}

void clockISR() {
  clockFlag = 1;
}

When making that modification, some part of code made no sense to me, may be I'm missing something,

if ((now - lastInterrupt) > cycleTime)
  {
    finalValue = (value * sign) / 100.00;
    currentBit = 0;
    value = 0;
    sign = 1;
    newValue = 1;
  }
  else if (currentBit < 16 )
  {

    if (dataIn == 0)
    {
      if (currentBit < 16) {
        value |= 1 << currentBit;
      }
      else if (currentBit == 20) {
        sign = -1;
      }

    }

This is part of Original code (inside decode() function,

There's a condition to check currentBit is less than 16, and inside that condition there's another condition to check whether currentBit is 20. currentBit is not volatile, so there's no way currentBit can change to 20 inside that if(currrentBit < 16) condition.

Am I wrong here?

About decoding code inside ISR, can you give me any example please???

You are correct about the logic. Probably, this should be changed:

else if (currentBit < 16 )

How many bits are you expecting from the device. 20 or more ?

The original code for the caliper software includes this warning: GitHub - MakingStuffChannel/DigitalCalipers: Source code and other files for reading values from digital calipers. See the project page for complete write up at http://makingstuff.info/Projects/Digital_Calipers. Note item 3 about digitalRead.

You appear to be violating this:

dataIn = digitalRead(dataPin) + digitalRead(dataPin) + digitalRead(dataPin);

I reviewed the video and original code here:
https://www.youtube.com/watch?v=5Us-V6410j8

The original prototype Arduino code doesn't truly work. While it does use interrupts, it doesn't use them appropriately to ensure the code always works - especially for uses that slightly differ from the provided prototype code.
The code needs to be redone to do the decoding in the ISR.
Until that is done it will always be very fragile and only "work" in certain use cases & environments.

IMO, that is not a reasonable implementation as I believe that code should "just work" and ALWAYS work.

Once the decoding is done in the ISR, anything can be done in the loop() code.
This includes using any output interface including i2c on an LCD device.

Without redoing the code to put the decoding into the ISR trying to get it to work 100% reliably is like trying to punch you way out of a paper bag. (you just can't do it)

There are comments in code and on the github page that are simply not correct like:

The ISR Can't handle the arduino command millis()

The ISR obviously can't loop on its return value but there is no issue calling millis() in an ISR.

There is also lots of stuff about how other interrupts like Serial output and digitalRead() or digitalWrite() overhead can cause issues. The real issue is the way the code is doing the decoding in loop which depends on very fast loop() processing which is not the way to do this type of coding which has very time critical needs.

And what the heck is this?

  dataIn = digitalRead(dataPin) + digitalRead(dataPin) + digitalRead(dataPin);

Each digitalRead() adds some timing overhead. If that made something work, you have a h/w issue like a slew rate issue on a data signal that is too slow for your interface h/w or a wiring issue (wires too long?) and likely need to deal with those issues appropriately in h/w.

My recommendation is to stop messing around and just fix the code to really work by moving the decoding into the ISR.
Once that is done the code will be bulletproof and just work.

BTW,
If you want to see LCD overhead, you can run the LCDiSpeed sketch included in the hd44780 library to see how long each byte time is for the LCD and library you are using.
The time for a setCursor() is the same for a single character so you can calculate how long it takes to print your output.

The hd44780_pinIO i/o class is quite a bit faster than the LiquidCrystal class.

--- bill

There are total 24 bits in each packet, firstt bit is always 1 and it's not a bit needed to calculate reading, next 21 bits contain the reading (from bit 2 to bit 21 in the order of LSB to MSB). 22 nd bit is sign bit, 0 for positive 1 for negative.

From viewing caliper data on the DSO, each data packet arrives in around 200 ms and duration of each packet(24 clock pulse) is 16 ms.

Some other threads found online have some mistakes, they say bit 21 is sign bit, but if you check their code bit 22 is the sigh bit

He ignores first bit, read remaining 23 bits(bit 2 to 24), and he uses first 20 bits (i < 20, from bit 2 to bit 21) to calculate value, and bit 22 (i == 20, bit 22) to find sign.

Another example

both thread says bit 21 is sign bits

This thread give more accurate info on bits

I'm guessing some of the off by one issues might be due to not counting bits properly.
Even you have inconsistencies in your text above.
i.e.

There are total 24 bits in each packet ...

and then:

He ignores first bit, read remaining 23 bits(bit 2 to 24) ...

If there are 24 bits then there is no bit 24 as the 24 bits will bit 0 to bit 23.

But none of that has anything to do with where/when the decoding is done.
The decoding needs to be done in the ISR in order to make the code bulletproof and immune to loop() timing and jitter.

--- bill

What I meant is there are 24 BITS from caliper(BIT 1, BIT 2,...., BIT 24),

He is not reading BIT 1 since it'll always be ONE, only reads next 23 BITS (from BIT 2 to BIT 24)

In his code, condition for sign bit

if (i==20) {

        sign=-1;
}

BITS from caliper : 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24
i : 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22

Since BIT 1 from caliper is not read, i = 0, reads BIT 2, i = 1 ==> BIT 3, like wise i = 20 ==> BIT 22, so 22 nd bit is the sign bit

Here's my caliper out in binary as well as Decimal, here's I am reading all 24 BITS from Caliper(see attached image)

From looking at this site: Blog of Wei-Hsiung Huang: Using Digital Caliper For Digital Read Out (DRO) Applications

The protocol appears to use a start bit followed by 19 bits of data and then a sign bit, then 2 unused bits, then a bit to indicate mm/Inches.

Doesn't seem that complicated.

The complexity seems to be in the using of inconsistent numbering indexes and bit numbers.
This seems to create confusion and, IMO, is a definite recipe for disaster.

While there are 24 bits of information in the "packet", If I were writing the code, I'd call and refer to the bits inside the packet what they really are for clarity.

  • Start bit
  • data bits 0 to 18
  • sign bit
  • 2 unused bits
  • mm/inches mode bit

and I would not start numbering the 24 protocol bits in the protocol "packet" at 1

But all of this is still outside the issue of making the code reliable which requires moving the decoding to inside the ISR vs trying to defer it to loop().

--- bill

I don't know how to use interrupts with Arduino.

I am almost finished with the Book "Make : AVR Programming",so i tried to code by directly accessing registers.

Here's the code that I come up with,

/*
 * caliper_pinChangeInterrupt.c
 *
 * Created: 15-05-2019 10:54:31 PM
 * Author : New User
 */ 

/************************************************************************/
/* NOTE : Every code description is based on the logic output of caliper. BC548 is used to level shift 1.5 V from caliper to 5V, so logic is reversed.
 Every HIGH from caliper read as LOW by controller and vice versa.                                                                     */
/************************************************************************/

#define F_CPU 16000000UL
#include <avr/io.h>
#include <util/delay.h>
#include <avr/interrupt.h>
#include "serial.h"
#include "millis.h"

#define LOW 0
#define HIGH 1

#define DATA PD3 // CLOCK Pin 
#define CLOCK PD2 // DATA Pin

volatile uint32_t timeNow = 0;
volatile uint8_t flag = 0;

uint8_t i;
volatile int8_t sign;

volatile uint32_t value; 
uint32_t previousValue = 0;

volatile float result;

volatile uint8_t data;

void initInterrupts(void);
void decode(void);

ISR(INT0_vect) // Run every time there is a change on Clock pin
{
 if( bit_is_clear(PIND, CLOCK) ) // if rising edge detected from caliper(logic reversed since BC548 used to raise voltage from 1.5Vto 5V), store current time
 {
 timeNow = millis();
 }
 if( bit_is_set(PIND, CLOCK) ) // detected falling edge, calculate duration of High pulse
 {
 if(millis() - timeNow > 100) // if duration greater that 100 ms, decode incoming pulses
 {
 decode();
 }
 }
}

int main(void)
{
    serial_init(); // init UART 9600 Baud rate @16 MHz
    initTimer(); // init timer0
 initInterrupts(); // External interrupt on PD2, configured for pinchange
    sei(); // enable inerrupts
 
    while (1)
    {
 if(flag == 1)
 {
 if(value != previousValue) // if a new value detected, print it out
 {
 int a = value; 
 
  serial_write('0' + (a / 10000));                 /* Ten-thousands */
  serial_write('0' + ((a / 1000) % 10));               /* Thousands */
  serial_write('0' + ((a / 100) % 10));                 /* Hundreds */
  serial_write('0' + ((a / 10) % 10));                      /* Tens */
  serial_write('0' + (a % 10));                             /* Ones */
  
  printString("\r\n");
  previousValue = value;
 }
 flag = 0;
 }
    }
 return 0;
}

void initInterrupts(void)
{
 EICRA |= (1 << ISC00); // Any logical change on INT0(PD2) generates an interrupt request.
 EIMSK |= (1 << INT0); // INT0: External Interrupt Request 0 Enable
}

/************************************************************************/
/* Function decode incoming data from caliper                           */
/************************************************************************/

void decode(void)
{
 sign=1;

 value=0;

 for (i=0;i<23;i++) {

 //EIMSK &= ~(1 << INT0);   // temporarily disable INT0 interrupt
 //sei();
 
 /************************************************************************/
 /*     LSB       MSB SIGN
 BIT from caliper : 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24
 i               :   not 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22
  needed                                                                   */
 /************************************************************************/
 
 
 while(PIND & (1 << CLOCK)); //wait until clock returns to HIGH- the first bit(BIT 1 from caliper) is not needed
 while( !(PIND & (1 << CLOCK)) ); //wait until clock returns to LOW
 data = ( (PIND & (1 << DATA)) + (PIND & (1 << DATA)) + (PIND & (1 << DATA)) ); // read data, sample 3 times to avoid glitches
 data = (data >> 3);

 if (data==LOW) { // if HIGH pulse received
 if (i<20) {
 value|= 1<<i;
 }

 if (i==20) { // BIT 22 of caliper, 0 for +ve and 1 for -ve
 sign=-1;
 }
 }
 }

 result=value; // it should be result = (value * sign) / 100.00;
 flag = 1;
 EIFR |= (1 << INTF0); // Clear INTF0: External Interrupt Flag 0
}

Is this what you meant?

I was little confused about declaring variables as volatile, so everything used inside ISR() declared as volatile.

I don't know how to print floating point & -ve values using UART (In Arduino Serial.print() takes care of everything)

It works, only issue is can't display -ve values and decimal places

23.36 mm is displayed as 02336

see attached image

I will write code for LCD and update

While that code does use interrupts. IMO, It isn't using them appropriately for this application and is a very bad example of how to use interrupts.
It only uses the interrupt to detect the start bit, then it will spin in the ISR inside of the decode() function to wait on and process all the bits of the packet.
The timing is not so critical and short that it is necessary to spin inside the ISR for the entire packet time.

Not only will it have interrupts blocked for a relatively long period of time (many ms) in the ISR while "decoding" is happening, it has the potential to hang forever should a clock edge ever be missed due to so some sort of issue.
Also, the way that the ISR code is written, it appears to drop every other data packet as it appears to wait for the inter packet pause.
After a sample is decoded, It appears that the next packet will be dropped looking for the end of a packet before it calls decode() again.

Due to the potential to hang, I think that code is actually worse than the less reliable code that did the decoding in loop() - at least that code didn't have the potential to hang.
The watchdog could be used to avoid the potential for a hang, but IMO, the real solution is much simpler and that is to simply to do the decoding of the sample value in the ISR 1 bit at time taking an interrupt on every clock transition.

What I was meaning by doing the decoding the ISR is that the code should take an interrupt on each clock transition, and the ISR should read the packet bits 1 bit per interrupt and create/construct the sample integer value on the fly.
i.e. there should never be any looping/polling of clock and and data bits during decoding.
decoding should be done on the fly 1 bit a time as the bits are being clocked from the calipers.
The ISR takes the clock interrupt, processes the single bit, then exits.

The code posted earlier worked this way, but it deferred to bit processing to loop() rather than just doing it in the ISR.

I would use two "sample" variables, one that is the last sample value, which could be a float and available to the foreground code, and then a working integer that is only used by the ISR and used to construct the incoming bits into the integer sample value. Once the full packet (all 24 bits has been received) the ISR working integer value is converted to the sample value inside the ISR taking into consideration if the units were mm or inches.

It isn't that difficult to do. The code will be very similar and much closer to the code that was calling decode from loop() rather than this latest code you posted.

Reading the data pin multiple times won't avoid glitches. What it might do is add a small amount of delay that might help when proper level shifters are not being used. The added time overhead can be enough to allow the voltage on the data signal to rise a bit higher when sampled which can help an out of spec signal show as a high.
i.e. out of spec signals can cause signals to appear to be low rather than high if read without inserting some amount of delay due to the slew rate of the signal.
I have seen this issue when interfacing 3v and 5v logic.
I even have a LCD library where I have added delays (a few microseconds) to work around this phenomena, I don't like it, and I don't recommend doing it, because in some cases the voltage can be off enough that highs are not always highs even with the added delay. But in my hd44780 library I did it to allow interfacing a 3v ESP8266 to a 5V lcd without having to use a level shifter.
(I always use them in my projects)

For something like this were the voltages are so far apart, I would definitely use a level shifter to make the h/w more robust and reliable.

--- bill