Simplification of programming.

This project is working, however I would like to know if it can be simplified, mainly the constant juste in the value of Timer1.initialize, without which, it fails to read the temperature or display.

#include <TM74HC595Display.h>
#include <TimerOne.h>
#include <OneWire.h>
#include <DallasTemperature.h>

int SCLK = 7;
int RCLK = 6;
int DIO = 5;
float temp1;
OneWire oneWire(4);
TM74HC595Display disp(SCLK, RCLK, DIO);

void setup() {
 
}

void loop() {
  float temp2;
  temperatura();
  Timer1.initialize(1060);
  Timer1.attachInterrupt(timerIsr);

  if (temp1 != temp2){
    temp2 = temp1;
    disp.dispFloat(temp2, 2);
    delay(10000);
    }
}

void timerIsr()
{
  disp.timerIsr();
}

void temperatura()
{
  Timer1.initialize(1000000);
  Timer1.attachInterrupt(timerIsr);
  DallasTemperature sensors(&oneWire);
  sensors.requestTemperatures();
  temp1 = (sensors.getTempCByIndex(0));
}

1060 microseconds is 1 millisecond. You want an interrupt every millisecond? there is then no time for loop.

I have no experience with Arduino LCD, so I do not know why you call it in an interrupt. Initialize the timer in setup() to activate the interrupt every N seconds and do not reinitialize it.

Never use delay(), unless it is a very small interval and/or the sketch is simple.

Timer1.initialize(1000000); get into the habit of being explicit Timer1.initialize(1000000ul);

.

Well, a little googling found a version of the TM74HC595Display.h library which has the .timerIsr() method. But nothing I can find online (in English) shows the .dispFloat() method. There may be a few pages in Russian which have that version of the library.

Additionally, the .timerIsr() method doesn't seem to be documented anywhere. Looking at the code, it looks like a "show" method which re-displays the current digits that the library has onto the display. Like maybe the display needs a constant refresh every millisecond or so?

I suspect that the reason for this causing problems is that the Dallas temperature sensor takes more than 1ms to read (up to one second if bus-powered) so the 1ms interrupts are disrupting communication with the sensor. This is a situation where it might make sense to detachInterrupt() for the period that the sensor is being read. Then attachInterrupt() again when finished. That may make the display go dark for 1 second. The actual time-critical parts of the sensor communication are shorter than 1ms, but you would have to hack your own version of the library to prevent that being interrupted by the display.

This is what I found with Google for a library with the .timerISR() and .dispFloat() methods.

https://github.com/AlexGyver/GyverMOD/blob/master/GyverMOD_libs/TM74HC595Display.cpp

I agree with MorganS the the resetting of Timer1.initialize() from 1060 to 1000000 in temperatura() is to suppress the interrupts during the sensor reading.

if (temp1 != temp2){
    temp2 = temp1;
    disp.dispFloat(temp2, 2);
    delay(10000);
    }

Equality between two floats is unusual, so this statement is mostly true, and there will be a 10 second delay in the code and the the flow in loop() looks like this

reset the timer interrupt period to one second and read the temperature reset the timer interrupt period to 1060 microseconds display the value wait ten seconds

During the delay(10000) the timer interrupt is triggering the display refresh.

An interrupt service routine is supposed to be fast - lightening fast. There is NOTHING fast about reading a DallasTemperature sensor. That is NOT what you should be doing in an interrupt service routine.

PaulS: An interrupt service routine is supposed to be fast - lightening fast. There is NOTHING fast about reading a DallasTemperature sensor. That is NOT what you should be doing in an interrupt service routine.

It isn't in the ISR. At least that detail seems to be correct in the original code.

It isn't in the ISR.

So it isn't. Sorry about the intrusion.

OK, so it seems that this unspecified LED display might be using a conventional 4-digit 7-segment type of display, sitting on top of 4 74HC595 serial-to-parallel shift registers. As such, it doesn’t need continuous refreshing to keep the display illuminated.

I still can’t find the library with the .dispFloat() method, but it seems that the .float_dot() method in the AlexGyver library might do the same thing. I have not downloaded the library to test, as I don’t have this display.

This should work, without ever needing the timerOne library or using any interrupts…

#include <TM74HC595Display.h>
#include <OneWire.h>
#include <DallasTemperature.h>

int SCLK = 7;
int RCLK = 6;
int DIO = 5;
float temp1;
OneWire oneWire(4);
DallasTemperature sensors(&oneWire); //move this here - only needs to be done once in the lifetime of the program
TM74HC595Display disp(SCLK, RCLK, DIO);

void setup() {

}

void loop() {
  static float temp2; //make this static, so it remembers the value for the next iteration of the loop
  temperatura();
  if (temp1 != temp2) {
    temp2 = temp1;
    disp.dispFloat(temp2, 2);  //are you sure you didn't intend to write disp.float_dot(temp2, 2); here?
    disp.timerIsr(); //this seems to be the poorly-named function which actually outputs 4 characters to the display
  }
  
  //other code can go here, or maybe just a delay if you really only want to see the display change once every 10 seconds
}

void temperatura()
{
  sensors.requestTemperatures();
  temp1 = sensors.getTempCByIndex(0);
}

disp.timerIsr(); //this seems to be the poorly-named function which actually outputs 4 characters to the display

I could not agree with you more. Particularly since the sketch is coupled with the Timer1 library. It was difficult for me to wrap my head around the timer1 interrupt ISR calling something named disp.timerISR().

If it were called disp.refresh() and was called every 1016 microseconds(or at whatever interval is needed by the particular display) by the Timer1 isr it would be more comprehensible.

Thank you all!