banging head situation ...

I wrote a small section of code to remove some zeros from log and save some formatting string, I know its not perfect, but it turns out to be far worse than that :
The code keeps behaving oddly. when put separately:

void digi2txt( char * buf, int dint,int ddec, int dkeep ) {
  Serial.print("\ndigi2txt: ");
  if( ddec < 1 ){
  sprintf(buf, "%01d", dint);
  Serial.print("=>");
  Serial.println( buf);
  return;
}
  sprintf(buf, "%%d.%%0%dd", dkeep);
  Serial.print( buf);
  sprintf(buf, buf, dint, ddec);
  Serial.print(" -> ");
  Serial.println( buf);
}

void setup()
{
  char buff[64] = "";
  Serial.begin(9600);
  Serial.print("0,0,1:");
  digi2txt(buff, 0,0,1);
  Serial.println(buff);
  Serial.print("100,0,1:");
  digi2txt(buff, 100,0,1);
  Serial.println(buff);
/* start looping if below not commented out.

  Serial.print("100,10,1:");
  digi2txt(buff, 100,1,1);
  Serial.println(buff);
*/
}

void loop(){};

The output looks like:

001:
digi2txt: =>0
0
100,0,1:
digi2txt: =>100
100

But when the commented out part is left active,
the output would become:

001:
digi2txt: => 0
001:
digi2txt: => 0
001:
...... and repeats endlessly

I have no idea what is wrong ? where is looping ?
Where should I start to understand what is going on here ?
Currently I am totally out of clue...
Any help ?

sorry, the bbcode edit script didn't load for some reason. reformatted

#7 below:

http://forum.arduino.cc/index.php/topic,148850.0.html

zcattacz:
I wrote a small section of code to remove some zeros from log and save some formatting string,

Show us a few examples of the sort of input and the sort of output you want to achieve.

...R

Use "blink without delay" ...

or read your PM

The sketch isn't looping. It's failing, and restarting. It can be difficult to see that a sketch fails and restarts. For that reason, it's often wise to Serial.print() an initialization message early in setup(), to indicate that the sketch has started. If the initialization message prints again, you can conclude that the sketch restarted.

I don't see much in the way of diagnostic messages in the sketch. Since it appears to restart, the first thing to do is to verify that it is indeed restarting, and identify the point where everything goes awry. Some simple Serial.print()'s will indicate that execution reaches particular points in the sketch. Identifying the point where messages stop printing will help localize the failure.

Here's a modified version of your code. I've added an initialization message, and deleted portions of the code that you don't seem concerned about. I've also added some diagnostic print messages that simply say that execution has reached particular points in the sketch, with Serial.flush()'es to make sure that the entire diagnostic output is printed before other code executes. Additions and deletions are marked. Finally, I've used the autoformat tool to clarify the program flow. Here's the code:

void digi2txt( char * buf, int dint,int ddec, int dkeep ) {
  Serial.println("<Got here: 2"); Serial.flush(); // ADDED
  Serial.print("\ndigi2txt: ");
  if( ddec < 1 ){
    sprintf(buf, "%01d", dint);
    Serial.print("=>");
    Serial.println( buf);
    return;
  }
  sprintf(buf, "%%d.%%0%dd", dkeep);
  Serial.println("<Got here: 3"); Serial.flush(); // ADDED 
  Serial.print( buf);
  Serial.println("<Got here: 4"); Serial.flush(); // ADDED
  sprintf(buf, buf, dint, ddec);
  Serial.println("<Got here: 5"); Serial.flush(); // ADDED
  Serial.print(" -> ");
  Serial.println( buf);
}

void setup()
{
  char buff[64] = "";
  Serial.begin(9600);
  Serial.println("Init."); // ADDED

// LINES DELETED HERE

  Serial.print("100,10,1:");
  digi2txt(buff, 100,1,1);
  Serial.println("<Got here: 1"); Serial.flush(); // ADDED
  Serial.flush();
  Serial.println(buff);
  Serial.println("Done."); // ADDED
}

void loop(){
};

As expected, the output repeats. Here's a sample of the output:

Init.
100,10,1:<Got here: 2

digi2txt: <Got here: 3
%d.%01d<Got here: 4
Init.
100,10,1:<Got here: 2

digi2txt: <Got here: 3

The initialization message prints repeatedly, so the sketch is starting over rather than looping. The sketch successfully prints "100,10,1:", but never reaches diagnostic message 1. The only statement between those is a call to digi2txt(), so it appears to fail inside that function.

Inside digi2txt(), diagnostic message 2 is printed, so the function does indeed get called. Message 3 prints, so the sprintf() that generates the format string appears to execute. Message 4 prints, so variable buf appears to print without incident. buf prints as "%d.%01d," which is what I'd expect from the format string and variable that generated it.

Message 5 doesn't print. There's only one statement between messages 4 and 5:  sprintf(buf, buf, dint, ddec);It looks like the sketch fails in that statement. This statement uses buf as both an input - the format string - and an output - the output string. That's certainly risky, and I don't google any documentation that says it's supported. Intuitively, I suspect that's what's causing this problem. A solution would be to establish a second character array to hold the format string, so that the sketch won't write over the format string before it's finished using it.

Trying the code with a second buffer to hold the format string yields this output:

Init.
100,10,1:<Got here: 2

digi2txt: <Got here: 3
%d.%01d<Got here: 4
<Got here: 5
 -> 100.1
<Got here: 1
100.1
Done.

The completion message printed, so the sketch executed fully. The final output looks like what I'd expect, given the inputs to the last sprintf() in digi2txt(). I can't tell if that's what you wanted, but this change does solve your primary complaint: that the code ran endlessly, and didn't yield a final output.

Here's the final code that generated that output. I'll leave it to you to fold it back into your sketch:

void digi2txt( char * buf, int dint,int ddec, int dkeep ) {
  char formatString[10];
  Serial.println("<Got here: 2"); Serial.flush(); // ADDED
  Serial.print("\ndigi2txt: ");
  if( ddec < 1 ){
    sprintf(buf, "%01d", dint);
    Serial.print("=>");
    Serial.println( buf);
    return;
  }
  sprintf(formatString, "%%d.%%0%dd", dkeep);
  Serial.println("<Got here: 3"); Serial.flush(); // ADDED 
  Serial.print( formatString);
  Serial.println("<Got here: 4"); Serial.flush(); // ADDED
  sprintf(buf, formatString, dint, ddec);
  Serial.println("<Got here: 5"); Serial.flush(); // ADDED
  Serial.print(" -> ");
  Serial.println( buf);
}

void setup()
{
  char buff[64] = "";
  Serial.begin(9600);
  Serial.println("Init."); // ADDED

// LINES DELETED HERE

  Serial.print("100,10,1:");
  digi2txt(buff, 100,1,1);
  Serial.println("<Got here: 1"); Serial.flush(); // ADDED
  Serial.flush();
  Serial.println(buff);
  Serial.println("Done.");
}

void loop(){
};

The strategy that you've chosen - generating a format string under program control - will be harder to troubleshoot than more direct methods. I'll admit that there's something very cool about writing code that writes code, but it has the disadvantage that it's very hard follow the program flow. I can't tell what you're trying to accomplish here. If you'll clarify that for us, we can probably suggest some simpler ways to do what you want.

I can't tell what you're trying to accomplish here.

OP: A simple comment that says

// Generate the format string

before the line of code that does so would have been immensely more helpful that leaving us to guess what you are doing.

tmd3 is spot-on in the assessment of the problem. Using buf as both input and output is not a good idea.

Thanks folks for the thorough analysis. I have been trying to achieve as much as possible with one UNO board by adding more sensor and log more data. The code is not big but its constantly running low in memory, forcing me to do everything possible to eliminate as many strings as I can.

But its a data logger right ? Its all about strings. I guess my biggest problem is that I wish the logger has a display to show real time readings. So I can't just log raw value without conversion, which if not used for logging still used for display.

Also the sensor has a long distance due to their position on the machine.
I tried to add some code to do filtering and moving average to smooth out the noise,
its working ok but has some problem with resting down to zero (4~20mA input).
The reading stuck at last value afte A1 is grounded.

// try moving avarage to smooth it
const uint8_t sample_size = 120;
uint16_t samples[sample_size];
uint8_t idx = 0;
const uint8_t sample_debounce = 5; // filter out sample inside  +/-5 range of lastest value.

....

int getSampleAvg() {
  int i = 0; long s = 0;
  while ( i < sample_size  ) {
    s += samples[i];
    i++;
  }
  return s / sample_size;
  
}

....

void setSample() {
  
  signed int sample = analogRead(A1) ;
  int idx0, sample0 ;
  if ( idx > 0 ) {
    idx0 = idx - 1 ;
  } else {
    idx0 = sample_size - 1 ;
  }
  // there is a randomly occasion 5unit quick bouncing. trying to filter it here.
  if (samples[idx0]) {
    sample0 = samples[idx0];
  } else {
    sample0 = 0;
  }
  if ( sample > sample0 + sample_debounce || sample < sample0 - sample_debounce ) {
    samples[idx] = sample;
    idx++;
    if ( idx >= sample_size  ) idx = 0;
  }
}

The code in question in this thread is for converting the "0.00" or "1.000","0.200" etc to "0", "1","0.2" they are bogus, occupying storage space and i/o time.
From what I read on the arduino wiki, I am under the impression that I should avoid float opr(now I am wondering if some careful use of the right amount of float operation is a good tradeoff considering the effort to solve all these small glitches), so ok, am using long numbers & x1000 times to compute the decimal places and sprintf("%d.%03d", int_part, dec_part) to get the reading. but its getting more complicated when its comes to improvement of the details like the "0"s. another example is the flickering of the screen. In order to avoid flicker, it seems that I have to always overwrite without clear(). like for 4 digit reading I have to always sprintf( "% 4s", digi_as_txt_to_show ) to ensure overwrite all old numbers on the screen.

The current code I am trying to improve:

...
// calculation
const PROGMEM uint16_t Rx100 = 23240 ; // 100 times of R
signed long Umv, Iua, Pg;
char Uv_txt[7];    // 9.999\0
char Ima_txt[9];   // 0.000999\0
char Pbar_txt[7];  // 99.99\0
char Tc_txt[7];    // 999.99\0
uint8_t freq_pin3 ;
uint8_t freq_pin2 ;

... 



void calculate() {
  
  uint16_t Uv_int, Uv_dec ;
  uint16_t Ima_int, Ima_dec ;
  uint16_t Pbar_int, Pbar_dec ;
  uint16_t Tc_int, Tc_dec;

  //some times memory low and these txt got garbled, seems to help.
  memset( Ima_txt, 0, sizeof(Ima_txt) );
  memset( Uv_txt, 0, sizeof(Uv_txt) );
  memset( Pbar_txt, 0, sizeof(Pbar_txt) );

  //unsigned long constant to force long computation
  Umv = getSampleAvg() * 5ul * 1000ul / 1023ul;
  Uv_int = Umv / 1000ul;
  Uv_dec = Umv - Uv_int * 1000ul;
  sprintf(Uv_txt, "%d.%03d", Uv_int, Uv_dec); // 9.999\0

  Iua = Umv * 1000ul * 100ul / Rx100 ;
  Ima_int = Iua / 1000ul;
  Ima_dec = Iua - Ima_int * 1000ul ;
  sprintf( Ima_txt , "%d.%03d", Ima_int, Ima_dec); // 0.000999\0

  if ( Iua <= 4000 ) {
    Pg = 0; Pbar_int = 0; Pbar_dec = 0; // no 4mA signal, check connection
  } else {
    Pg = ( Iua - 4 * 1000ul ) * 1000ul / ( 20 * 1000ul - 4 * 1000ul ) ; // 1000g = 1bar
    Pbar_int = Pg / 1000ul ; // when
    // the pressure switch has only 2 decimal places precision for bar., change the 10ul and the %02d at the same time!!
    Pbar_dec = (Pg - Pbar_int * 1000ul) / 10ul ;
  }
  sprintf( Pbar_txt, "%d.%02d", Pbar_int, Pbar_dec);  // 99.99\0
  
  float Tc;
  Tc = DS3231_get_treg();
  Tc_int = int(Tc) ;
  Tc_dec = Tc * 10 - Tc_int * 10 ;
  sprintf( Tc_txt, "%d.%01d", Tc_int, Tc_dec);  // 99.99\0

}

Is there any flexible ways to handle the decimal fraction to text formatting ?

Since I have 2 pulse input to count, speed becomes critical to me. I tried let it run as fast as possible.
I tried to structure it like this, not sure if this is the best way to do it.
What is the Maximum speed of sampling UNO is able to catch up without missing data points in a application like this? I am using the sdfat library. flush() every 30 write() request, the call write every second.
Currently I am only using the 1st pulse input. I got data points(missing time stamp) lost, I don't know if there is a fixed interval, at least > 2m.

// time marks for each task
long t0_sdwrite = 0 ;
long c0_sdflush = 0 ;
long t0_refreshlcd = 0 ;
long t0_pulsecount = 0 ;
const static PROGMEM uint16_t interval_sdflush = 30 ; // a count of sdwrite
const static PROGMEM uint16_t interval_sdwrite = 1000 ; 
const static PROGMEM uint16_t interval_refreshlcd = 350 ;  // this lcd is slooow & laggy.
const static PROGMEM uint16_t interval_pulsecount = 1000 ; // per second

...

void loop() {
  setSample( );

  if ( t0_pulsecount + interval_pulsecount < millis() ) {
    freq_pin3 = pulsecount_pin3;
    freq_pin2 = pulsecount_pin2;
    pulsecount_pin3 = 0;
    pulsecount_pin2 = 0;
    Serial.print("p");
    t0_pulsecount = millis();
  }

  if ( t0_sdwrite + interval_sdwrite < millis() ) {
    calculate();
    logMsg();
    c0_sdflush++;
    Serial.print("w");
    t0_sdwrite = millis();
  }

  if( c0_sdflush >= interval_sdflush ) {
    logFile.flush();
    Serial.println("F");
    c0_sdflush = 0;   
  }

  if ( t0_refreshlcd + interval_refreshlcd < millis() ) {
    calculate();
    refreshLCD();
    Serial.print("L");
    t0_refreshlcd = millis();
  }

}

Currently the performance I can live with but certainly not very good. Bearly usable with a second pulse. The tested input pulse is <100Hz. The pulse counting is done by below library. I thought this should works better than counting interrupt by myself ?

// UNO's pin2 pin3 is for hardware external interrupt,will give the max speed,remove all other pins
// https://github.com/GreyGnome/EnableInterrupt/wiki/SaveMemory#ATmega328
// enable interrrupt on port 2
//#define EI_NOTEXTERNAL
//#define EI_NOTINT0
//#define EI_NOTINT1
#define EI_NOTPINCHANGE
//#define EI_NOTPORTB
//#define EI_NOTPORTC
//#define EI_NOTPORTD
#define NEEDFORSPEED
#define INTERRUPT_FLAG_PIN3 pulsecount_pin3
#define INTERRUPT_FLAG_PIN2 pulsecount_pin2
#include <EnableInterrupt.h>

void setup(){
...
  // attach interrupt for pulse count
  pinMode(2, INPUT_PULLUP);
  enableInterruptFast(2, RISING );
  //pinMode(3, INPUT_PULLUP);
  enableInterruptFast(3, RISING);
...
}

Though I only use edge detection, I can't uncomment any EI_NOTPORT* or it will give me error. Not sure if it is the library's glitch or my misuse ?

As mentioned above, the string got all over the logmsg part.

I want to format string, but when I use F() or PROGMEM on the formatstring,
in sprintf(buff, "%formatstring", var1)
All I got is weird random garbled string. Some times the program behaves oddly like in this thread.
I wonder if there is any way or common technique to work around this ?

  • have flexible formating, but takes up less space in memory.
  • not too many coding for each and every specific variable format
    Now I stick to print() whenever possible. It feels much awkward than template strings. I guess there are better way, right?
void logMsg() {
  
  if ( !logFile ) openFile();
  logFile.print(getTime( DATE_TIME ));
  logFile.print(",");
  logFile.print(getTime( TIME_TIME ));
  logFile.print(",");
  logFile.print(Uv_txt);
  logFile.print(",");
  logFile.print(Ima_txt);
  logFile.print(",");
  logFile.print(Pbar_txt);
...

Thanks in advance.

But its a data logger right ? Its all about strings.

One at a time, yes.

I am under the impression that I should avoid float opr

In time-critical situations, yes. Data logging doesn't usually fit into that category.

now I am wondering if some careful use of the right amount of float operation is a good tradeoff considering the effort to solve all these small glitches

You haven't explained where the logged data is going to be used. If the data is going to be read by a PC, that is orders of magnitude faster at calculations, AND has a floating point unit, log raw data and let the PC massage it.

so ok, am using long numbers & x1000 times to compute the decimal places and sprintf("%d.%03d", int_part, dec_part) to get the reading. but its getting more complicated when its comes to improvement of the details like the "0"s.

dtostrf() seems to have addressed all the issues.

it seems that I have to always overwrite without clear(). like for 4 digit reading I have to always sprintf( "% 4s", digi_as_txt_to_show ) to ensure overwrite all old numbers on the screen.

Or just write some spaces after the value.

Is there any flexible ways to handle the decimal fraction to text formatting ?

dtostrf().

const static PROGMEM uint16_t interval_sdflush = 30 ; // a count of sdwrite
const static PROGMEM uint16_t interval_sdwrite = 1000 ; 
const static PROGMEM uint16_t interval_refreshlcd = 350 ;  // this lcd is slooow & laggy.
const static PROGMEM uint16_t interval_pulsecount = 1000 ; // per second

None of those values needs a int to hold them. They are all byte sized. You are slowing the code down having to fetch those values from flash, instead of SRAM. To save 4 bytes of SRAM, is it worth it?

I want to format string, but when I use F() or PROGMEM on the formatstring,
in sprintf(buff, "%formatstring", var1)
All I got is weird random garbled string. Some times the program behaves oddly like in this thread.

Of course. Because different code is needed to access data in PROGMEM than is needed to access data in SRAM. The sprintf() function can't access data from PROGMEM.

Now I stick to print() whenever possible. It feels much awkward than template strings

We can't tell anything from that snippet. In general, format strings are small, and not worth the effort of keeping out of SRAM.

Post ALL of your code. Use Reply, not the Quick Reply field, and use the Additional Options link below the text entry area to attach your code, if it exceeds the 9500 character limit.

There may be other things you are doing that can be memory-optimized.

zcattacz:
The code in question in this thread is for converting the "0.00" or "1.000","0.200" etc to "0", "1","0.2" they are bogus, occupying storage space and i/o time.

I have a short attention span.

How can it be necessary to write two or three screen's full of text to convey the essence of this problem - which is in these two lines.

Why not traverse the string from left to right. Discard all 0s until you come to a larger number or a full-stop.
Then keep all the remaining characters.
Then with the shorter string do the same from the other end.

...R

PaulS:

const static PROGMEM uint16_t interval_sdflush = 30 ; // a count of sdwrite

const static PROGMEM uint16_t interval_sdwrite = 1000 ;
const static PROGMEM uint16_t interval_refreshlcd = 350 ;  // this lcd is slooow & laggy.
const static PROGMEM uint16_t interval_pulsecount = 1000 ; // per second



None of those values needs a int to hold them. They are all byte sized.

You seem to have access to much larger "bytes" than the rest of us do. My bytes can only hold values up to 255. So how, exactly, are you going to put 350, or 1000 into a byte???

Regards,
Ray L.

You seem to have access to much larger "bytes" than the rest of us do. My bytes can only hold values up to 255. So how, exactly, are you going to put 350, or 1000 into a byte???

Smartass 8). Clearly, I hadn't had enough tea yet.

PaulS:
tmd3 is spot-on ...

Thanks for saying so. I was hoping, instead, to be spot-on about the value of diagnostic messages for troubleshooting code. I think I inadvertently omitted a short speech about that.

zcattacz:
... its constantly running low in memory ...

I can't tell what kind of memory is running short. It may be that the SD is filling up, or it may be that the RAM is running out while the code accumulates data to write to the SD. Looking at the snippets, it seems that a lot of things are done the hard way, and using quite a bit of RAM in the process. 120 bytes alone are dedicated to a moving average calculation.

So, what kind of memory is running low? RAM, or SD?