Timing issues ?

Hello

Im using the following function that lets me shift our 64bits of data:

void setOutputs(unsigned long val_one, unsigned long val_two) { //Function to shift out 2 x 32bit fast enough to prevent flicker!

  // ------------WARNING!--------------------
  // This functions operates directly on ports, not via digitalWrite()
  // because digitalWrite() would be to slow, and display would flicker
  // if different pins are used, you maybe hav to change the variable "thePort"
  // to the matching I/O port letter of the controller!
 
  Serial.print("START:");
  Serial.print(millis());
  
  
  digitalWrite(OE, LOW); //Disable Outputs to prevent flicker

  //Send first 32-bit variable value

  for ( int i = 0; i < 32; i++) {
    thePort &= ~_BV(DATA);  //Data LOW
    if ( bitRead(val_one, i) == 1) {
      thePort |= _BV(DATA);  //Data HIGH
    }
    thePort |= _BV(CLK);  //CLK HIGH
    thePort &= ~_BV(CLK); //CLK LOW
  }

  //Send second 32-bit variable value
  for ( int i = 0; i < 32; i++) {
    thePort &= ~_BV(DATA);  //Data LOW
    if ( bitRead(val_two, i) == 1) {
      thePort |= _BV(DATA);  //Data HIGH
    }
    thePort |= _BV(CLK);  //CLK HIGH
    thePort &= ~_BV(CLK); //CLK LOW
  }
  digitalWrite(OE, HIGH); //Enable Outputs


    Serial.print("END:");
  Serial.print(millis());
}

In normal conditions, the START and END time differs in around 1ms. In some cases it differs in 20ms, which will cause major flicker on the outputs.

I think it will be, because other parts of the code take longer or something is interrupting the function.

Is there a way to find out what causes the slowdown?
Is there a way to code a function in a way that it cant be interrupted by anything?

Thanks

You could make your critical code an ISR and use a software interrupt to execute it.

You need to post the complete program so we can see how all the variables are defined.

And it seems likely to me that the problem is caused by something going on elsewhere in your program.

In this line

for ( int i = 0; i < 32; i++) {

why are you wasting SRAM and time using an int when a byte is sufficient?

Don’t measure the time as you are doing. Just save the value of millis() at the start (without printing anything) and save it again at the end. Then print the difference. And using micros() will give you a more accurate measure.

…R

Robin2:
You need to post the complete program so we can see how all the variables are defined.

And it seems likely to me that the problem is caused by something going on elsewhere in your program.

In this line

for ( int i = 0; i < 32; i++) {

why are you wasting SRAM and time using an int when a byte is sufficient?

Oh, did not know that byte would work! All the examples with for that i have seen, are made with int! Will change in all my for loops!

Attached is my code. I think it could be a issue with some of the libraries i use but sadly i have not found out yet.

arduino_forum.ino (24.8 KB)

This probably isn't the issue, but ...

bool menu_true = 0;

...
  if  (menu_true == 1 && millis() - last_menu > menu_timeout || menu_reset == 1) {

Booleans are technically true or false, not 1 or 0. You should be writing:

bool menu_true = false;  // the default anyway
...
  if  (menu_true && millis() - last_menu > menu_timeout || menu_reset) {

Plus I hope you are sure whether &&, -, > and || have the right priority. If not, I suggest parentheses.

  if (enTsEval == 2);{

enTimeSync =1;
  }

This looks wrong, somehow.

You might be flooding the serial output buffer if the function in the opening post is called constantly. Eventually the buffer will be full and the serial.print will block till there is space again.

Can't look at your attached code at the moment.

That's an awful long program to expect people to study.

My guess is that sometimes the Timer interrupt fires during the function setOutputs().

I don't know if it would be acceptable to disable interrupts for the duration of setOutputs() - that may solve one problem and cause another. Maybe just use millis() to manage timing in place of the TimerOne library.

And as @sterretje says, flooding the serial output buffer is another possible cause.

...R

sterretje:
You might be flooding the serial output buffer if the function in the opening post is called constantly. Eventually the buffer will be full and the serial.print will block till there is space again.

Can't look at your attached code at the moment.

No, sorry i explained it wrong sorry! The serial.print was only there to debug! The effect occours without the time print too, i just added it do confirm the shifting takes longer than usual.

Robin2:
I don't know if it would be acceptable to disable interrupts for the duration of setOutputs() - that may solve one problem and cause another. Maybe just use millis() to manage timing in place of the TimerOne library.

And as @sterretje says, flooding the serial output buffer is another possible cause.

...R

Hm, TimerOne is needed for the rotary encorder library, if i get rid of TimerOne, i have to remove that library too.

Might look wrong, but its not. If the EEPROM is 1, enTimeSync becomes 0, and if EEPROM is 2, enTimeSync becomes 1 (enabled).

Some details about the project (mabye helpfull):

The device is a nixie clock, if left alone, it just uses "set output" each second to change the seconds display. There is also the possibility to change parameters by a rotary encoder.
Each minute, a NMEA device will send over the current time (emulated by NTP, but code is 1:1 NMEA), if the rtc and gps differ, the time will update. A Timestamp is send each second, but only each minute i do the comparison. The only really critical part is the shifting, because if it takes to lon, you will see a "flicker (on/off)" on all display, when one value changes. I found out that shifting must be shorter than 1mS, to be invisible.

sgt_johnny:
Might look wrong, but its not. If the EEPROM is 1, enTimeSync becomes 0, and if EEPROM is 2, enTimeSync becomes 1 (enabled).

Yes it is.

  if (enTsEval == 2);{
  enTimeSync =1;
  }

So, breaking it down:

 if (enTsEval == 2);

If nTsEval == 2 do nothing.

Unconditionally:

  enTimeSync =1;

That is, always, set enTimeSync to be 1.

SoftwareSerial is a real CPU hog. It disables interrupts for the entire time that a character is being received or send, about 1ms. It could be delaying your sketch when NMEA characters arrive, and it may be interfering with the TIMER1 ISR. I strongly recommend AltSoftSerial, which only works on pins 8 & 9 (currently your PWR and BUZZER pins). I hope you can switch pins, because it is much, much more efficient.

If you can't switch pins, you should consider my NeoSWSerial library. It is almost as efficient as AltSoftSerial, but it works on any two pins. It is limited to baud rates 9600, 19200 and 38400, so it would work for the NMEA stream.

And while I'm at it, you might take a look at my NeoGPS library. It is much smaller and faster than your current library, probably taking less than 0.5ms to parse the date and time from an RMC sentence, compared to 1.4ms for TinyGPS++. When configured to parse only the date and time, NeoGPS would only use less than 20 bytes of RAM instead of 240.

But wait, there's more! :slight_smile: You could hook NeoGPS parsing to the NMEA RX interrupt. This would allow the date and time to be accumulated in the background, without slowing down your sketch (noticeably). In this mode, each character is parsed as it arrives. This spreads the 0.5ms CPU parsing time out over the 70ms it takes to receive the entire RMC sentence. This allows your refresh loop to run much more smoothly.

If you'd be interested, I could post a NeoGPS version of your sketch. The non-interrupt version is about 1 K less program space, adn the interrupt-driven version is about 2K more program space (due to NeoSWSerial), but 150 fewer bytes of RAM. And about 1ms faster on the parsing, as I mentioned.

Cheers,
/dev

Thank you very much for pointing that out, that was one of the coding errors the compiler won't detect! Thanks!

/dev:
SoftwareSerial is a real CPU hog. ....
If you'd be interested, I could post a NeoGPS version of your sketch.
Cheers,
/dev

I figured that out today, i continously started to disable features until it run stable, the GPS thing did the job.

I used TinyGPS because it was the first i found, but it would be very good if there is a better one!
I would really thankful if you could send me the Version with NeoGPS! The pins cant be changed, since this is a finished pc board :confused:

And the NMEA device can only transmit, it has only one signal pin. If thats in anyway helpfull

But here are some observations:

  • unsigned long is not guaranteed to be 32 bits. If you want to ensure a 32 bit value, use uint32_t
    While this is not currently an issue since AVR is being used and on AVR unsigned long is 32 bits it is something to keep in mind for the future for portability to other processors.

  • it usually generates better/faster code to shift the data and always test a constant bit rather than test a moving bit on constant value.
    i.e. write the test your self to test bit 0 of the data and then shift the value to the right each iteration rather than use the bitRead() macro.

In terms of really making it faster, why not use the SPI h/w? That is what it does and you won't have to worry about interrupts. It will clock out the data much faster than you could ever do it in s/w.
This assumes you can switch to using SPI pins which are digital pins 11 and 13.

But my overall suspicion at this point is that the timing of pushing the bits out is not the real issue especially given you said are seeing jitter of 20 ms. That kind of jitter is not due to the routine shifting out the bits.
It is very likely due to all the serial output being done.

You need to be looking at the timing from an overall system perspective vs assuming there is an issue in any one particular place.
My saying from long ago is "Better algorithms beats faster code". The idea is that things need to be looked at from a system level rather than trying to optimize individual components because often optimizing some code to be faster - even if it is much faster - doesn't make enough difference in the overall system performance.
When it comes to things like timing, it is like a chain; the slowest link can slow up the entire system.
So in order to fix something like this you must understand the entire system to know where the time is being lost and potentially make changes to the overall architecture to accommodate the timing requirements.

One thing that you should consider is switching to using the Time library so that you are not hammering the I2C bus with constant requests to the RTC. The Time library will track time on its own and you can set up a RTC synchronizer routine to keep it in sync. The value of this is that the actual RTC will only be accessed every few minutes vs constantly. This will free up cycles.
Another advantage of using the Time library is that you could update the code to use the Timezone library so that time could automatically be updated for DST/Summer time changes.
I'd recommend taking a look at using Time, DS3232RTC, and TimeZone as they all work together. (it works on ds3231 chips)

Some quick and dirty things you can do:

  • speed up the I2C interface to use 400khz for a slight bump.
  • set the baud rate WAY up on h/w Serial to like 115200.
    This is a problem in your code since you have the baud rate at only 9600 are tossing out lots of messages.
    This will block the system once the output buffer fills and you are definitely outputting more than the buffer holds in several places.
  • output less to Serial and possibly use some conditionals to allow easily disabling most of the output.

--- bill

Assuming you have Wifi available where this clock will be living, another option to set the time automatically would be to use a ESP8266 module and then use WiFi to get the time from an NTP server rather than use GPS. The ESP modules are very cheap (like around $2-3 USD). And you could hook it to the AVR serial port and spit out the time messages in whatever format you want however often you want.
It is easy to code since it also can use Arduino and there are lots of examples of how to use NTP and Wifi manager to allow automatic configuration over Wifi.

It is a 3v processor so you have to be a bit careful, but since all you would need is serial output feeding into the AVR, you can hook it up directly as long as the AVR won't ever drive that pin HIGH (which it shouldn't).
The 3v output signals from the ESP will be high enough for the AVR to see it as a high.

If you wanted, you could do things differently by having the ESP do more.
You would change the system to use a web page for setting the time as you could do all that in the ESP module.
i.e. have the ESP module send local time to the AVR and have the ESP do the GMT to local time offsets/conversions and DST changes. The AVR code can then be really dumb and simply display the time it gets from the RTC which is updated periodically by the ESP when the ESP sends the time to the AVR over the serial port.
All the time offsets, and time setting/changes would be handled by the ESP.

--- bill

bperrybap:
Assuming you have Wifi available where this clock will be living, another option to set the time automatically would be to use a ESP8266 module and then use WiFi to get the time from an NTP server rather than use GPS.

The user should have the freedom to select any standart NMEA Reciever. Currently i tested a "GPS MOUSE" as well a device, specifically made for clocks like these, that emulates NTP to a NMEA signal to support older clocks, where WIFI wasnt a option.

The Clock itself has no wifi. And should be compatible with NMEA Recievers that put out a 5V Signal.

I was just trying to offer you suggestions/alternatives for getting your system up and working reliably as currently it sounds like the s/w you have seems to be having some issues because it appears to have some s/w design & architecture issues.

  • frequent polling of RTC over i2c bus
  • use of softserial port for NMEA messages
  • outputting lots of serial messages on h/w serial port at a relatively slow baud rate

Those issues will have be addressed regardless of the automatic time source.

sgt_johnny:
The Clock itself has no wifi. And should be compatible with NMEA Recievers that put out a 5V Signal.

That is why I suggested that you could use a ESP8266 module instead of a GPS module as the ESP includes the wifi.
You write an Arduino sketch that runs inside the ESP8266 module that gets the time from an NTP server using WiFi and then spits out whatever you want on a serial port that feeds to the AVR, including NMEA messages if that is what you want.

While the ESP is 3v, you can cheat by feeding the 3v serial signal from the ESP directly to the AVR serial input without a level shifter. (as long at the AVR will always keep that pin as an input)

There are several advantage of using an ESP module over a GPS module.
One being that a clear GPS signal is not available, but perhaps wifi access is.
It is going to be cheaper in most cases and they are quite small.
Also you control the s/w in it so it can do lots of other things like automatic DST/summer time changes. Or provide a web interface to be able to set/configure features.
In other words the ESP could play games by shifting the time being reported to handle the DST changes.
And if you create a web page on the ESP to configure it, all configuration could be done through the web page wirelessly rather than through physical buttons/controls.
If the ESP were handling the timezone, and DST/summertime time changes, the clock essentially becomes a set/configure and forget since there should be no need to have to set it once it is configured.

— bill

I would really thankful if you could send me the Version with NeoGPS!

Attached. It compiles, so it must work. :wink: You will also need the NeoSWSerial library.

One thing that you should consider is… not hammering the I2C bus

+1! I don’t think this is causing your problem, but it your sketch is very inefficient.

For your application, it would be better to use a time structure+seconds count in RAM. You keep both items, because the structure has the pieces you display, and the seconds count is a long integer that increments for a seconds tick (add 1 every 1000ms) or timezone offset calculation (add/subtract hours * 60 * 60).

NeoGPS uses a faster, slimmed-down version of the Time library, although you can use both libraries. There are methods in both libraries for converting seconds to a structure, and vice versa. The attached program updates a global fix which contains the fix.dateTime time structure. It also saves the corresponding seconds count in the variable gpsTime.

There are 3 things you need to implement:

  1. When a new GPS time structure arrives, it is used to set the seconds count. But if it doesn’t arrive within 1000ms, you should increment it yourself. Then convert the seconds count back to the time structure and use those pieces to set the display. (The Time library does this for you with the setTime and now() functions.)

  2. If no GPS time has been received for “a while”, you should re-sync the seconds count with the RTC (10 minutes? 1 hour? Not every second!) . (The Time library does this for you with the setSyncProvider function, once every 5 minutes.)

  3. Likewise, when a GPS time is received and it has been “a long time” since you set the RTC (1 day? 1 month? Ever?), you should set the RTC.

These three things will dramatically reduce the I2C traffic. Once these are in place, you could look at ways to calibrate the millis() from the RTC and/or GPS. micros() might come in handy for knowing that your Arduino is running at 1000056us for each “real world” second.

Cheers,
/dev


P.S. Here’s a snippet from the NMEAtimezone example. It shows how to offset for a local timezone, and how to test for DST in the USA:

    //  USA Daylight Savings Time rule (calculated once per reset and year!)
    static NeoGPS::time_t  changeover;
    static NeoGPS::clock_t springForward, fallBack;

    if ((springForward == 0) || (changeover.year != fix.dateTime.year)) {
      changeover.year    = fix.dateTime.year;
      changeover.month   = 3;
      changeover.date    = 14; // latest 2nd Sunday
      changeover.hours   = 2;
      changeover.minutes = 0;
      changeover.seconds = 0;
      changeover.set_day();
      // Step back to the 2nd Sunday, if day != SUNDAY
      changeover.date -= (changeover.day - NeoGPS::time_t::SUNDAY);
      springForward = (NeoGPS::clock_t) changeover;

      changeover.month   = 11;
      changeover.date    = 7; // latest 1st Sunday
      changeover.set_day();
      // Step back to the 1st Sunday, if day != SUNDAY
      changeover.date -= (changeover.day - NeoGPS::time_t::SUNDAY);
      fallBack = (NeoGPS::clock_t) changeover;
    }

    // Set these values to the offset of your timezone from GMT
    int32_t zone_hours   = -5L; // EST
    if ((springForward <= seconds) && (seconds < fallBack))
      zone_hours++;

    static const int32_t         zone_minutes =  0L; // usually zero
    static const NeoGPS::clock_t zone_offset  =
                      zone_hours   * NeoGPS::SECONDS_PER_HOUR +
                      zone_minutes * NeoGPS::SECONDS_PER_MINUTE;

    NeoGPS::time_t localTime( seconds + zone_offset ); // convert seconds back to a local time struct

sgt_johnny.ino (25.9 KB)

bperrybap:
That is why I suggested that you could use a ESP8266 module instead of a GPS module as the ESP includes the wifi.

I already have a device that works on a EPS8266 and emulates NMEA signal, its called nwts :

I will look into your other suggestions and will make my code better! thank you very much

P:\Arduino\libraries\NeoGPS\src/Location.h:33:5: note: in expansion of macro 'CONST_CLA

/dev:
Attached. It compiles, so it must work. :wink: You will also need the NeoSWSerial library.
+1! I don't think this is causing your problem, but it your sketch is very inefficient.

For your application, it would be better to use a time structure+seconds count in RAM. You keep both items, because the structure has the pieces you display, and the seconds count is a long integer that increments for a seconds tick (add 1 every 1000ms) or timezone offset calculation (add/subtract hours * 60 * 60).

Hello, thank you very much for the example and your comments. I downloaded both libraries from your github. But cant compile, the messages look like:

P:\Arduino\libraries\NeoGPS\src/NeoGPS_cfg.h:81:35: error: 'constexpr' does not name a type

#define CONST_CLASS_DATA static constexpr

^

What could be the problem?

I will look into the other thing you told me

But cant compile, the messages look like:

error: 'constexpr' does not name a type

What version of the IDE are you using, and what board were you building for?

This has been an annoying difference between IDE versions, Arduino.org vs. Arduino.cc, and AVR GCC compiler versions. Grrr.

Thanks,
/dev

/dev:
What version of the IDE are you using, and what board were you building for?

This has been an annoying difference between IDE versions, Arduino.org vs. Arduino.cc, and AVR GCC compiler versions. Grrr.

Thanks,
/dev

I use Arduino 1.6.12 IDE and are compiling for Arduino Uno.

I updated to the 1.8.0 and it now works!