An array rounds a string weirdly.

Alright, I have this code and basically it takes sensor data, puts it into an array and then writes it. For some reason that is beyond me, data from the pH sensor is rounded. I've narrowed down the problem to when I take out the println(phdata[1]) and myfile.println(phdata[1]) that fixes it. I have no idea why this fixes it but there ya go. Here's my code.

Bump

Post the code here and in code tags. Not on pastebin.

LightuC:
Post the code here and in code tags. Not on pastebin.

Alright, here. Code pasted in code tags

//******** Libraries ***********
#include <SparkFun_Qwiic_Relay.h>
#include <U8glib.h>
#include "SparkFun_Qwiic_Relay.h"
#include "U8glib.h"
#include <Wire.h>
#include <SoftwareSerial.h>
#define rx 2
#define tx 3
#include "TSYS01.h"
#include "MS5837.h"
#define RELAY_ADDR 0x18
#include <SPI.h>
#include <SD.h>
//*******************************
 
TSYS01 sensor1;
MS5837 sensor0;
 
SoftwareSerial myserial(rx, tx);
Qwiic_Relay relay(RELAY_ADDR);
File myFile;
 
int temppos = 0;
int depthpos = 0;
int phpos = 0;
float depthdata [15] = {};
float tempdata [15] = {};
float phdata [15] = {};
float relaytime = 0;
int sensortiming = 0;
String inputstring = "";
String sensorstring = "";
boolean input_string_complete = false;
boolean sensor_string_complete = false;  
float pH;
 
void setup() {
 
  SD.remove("Output.txt");
  Serial.begin(9600);
  myserial.begin(9600);
  inputstring.reserve(10);
  sensorstring.reserve(30);  
  Wire.begin();
 
  sensor1.init();
  while (!sensor0.init()) {
    Serial.println("Init failed!");
    Serial.println("Are SDA/SCL connected correctly?");
    Serial.println("Blue Robotics Bar30: White=SDA, Green=SCL");
    Serial.println("\n\n\n");
    delay(5000);
  }
 
  if(!relay.begin()) {
 
    Serial.println("Check connections to Qwiic Relay.");
 
  } else {
 
    Serial.println("Ready to flip some switches.");
 
  float version = relay.singleRelayVersion();
 
  Serial.print("Firmware Version: ");
 
  Serial.println(version);
 
  }
 
 
  Serial.print("Initializing SD card...");
 
  if (!SD.begin(4)) {
    Serial.println("initialization failed!");
    while (1);
  }
  Serial.println("initialization done.");
 
  sensor0.setModel(MS5837::MS5837_30BA);
  sensor0.setFluidDensity(997); // kg/m^3 (freshwater, 1029 for seawater)
}
 
void serialEvent() {
  inputstring = Serial.readStringUntil(13);
  input_string_complete = true;
}
 
void loop() {
 
  if (relaytime >= 10) {
 
    relay.turnRelayOn();
 
    delay(1000);
 
    relay.turnRelayOff();
 
    delay(1000);
 
    relay.turnRelayOn();
 
    delay(1000);
 
    relay.turnRelayOff();
 
    relaytime = -10; //The time it takes for the relay to execute again (in seconds)
 
    myFile = SD.open("Output.txt", FILE_WRITE);
    //Serial.println(phdata [1]);
    Serial.println(tempdata [1]);
    Serial.println(depthdata [1]);
    Serial.println("");
    //myFile.println(phdata [1]);
    myFile.println(tempdata [1]);
    myFile.println(depthdata [1]);
    myFile.println("");
    myFile.close();
 
  }
 
 
  if (sensortiming == 1000) {
    sensor1.read();
    sensor0.read();
 
    Serial.print("Depth: ");
    Serial.print(sensor0.depth());
    Serial.println(" m");
    depthdata [depthpos] = sensor0.depth();
    depthpos = depthpos+1;
   
    Serial.print("Temperature: ");
    Serial.print(sensor1.temperature());
    Serial.println(" deg C");
    tempdata [temppos]= sensor1.temperature();
    temppos = temppos+1;  
 
    Serial.println("");
    sensortiming = 0;
  }
 
 
  if (input_string_complete == true) {
    myserial.print(inputstring);
    myserial.print('\r');
    inputstring = "";
    input_string_complete = false;
  }
 
  if (myserial.available() > 0) {
    char inchar = (char)myserial.read();
    sensorstring += inchar;
    if (inchar == '\r') {
      sensor_string_complete = true;
    }
  }
 
  if (sensor_string_complete == true) {
    if (isdigit(sensorstring[0])) {
      pH = sensorstring.toFloat();
    }
    phdata [phpos] = pH;
    Serial.println(pH);
    sensorstring = "";
    sensor_string_complete = false;
    phpos = phpos+1;
  }
 
  delay(1);
  sensortiming = sensortiming+1;
  relaytime = relaytime+0.001;
}
  1. Your array sizes are magic numbers. See #7 below for more.

  2. It will be a good idea to begin the SD card before attempting to delete any files on it.

  3. I've never seen readStringUntil() used inside serialEvent() I'm surprised that it works at all.

readStringUntil() is a blunt tool. serialEvent() isn't much sharper. They are best treated as "seemed like a good idea at the time" and should not be used in anything except small demonstrations.

You have the right idea in the myserial section later down in loop(). Use that method for both serial inputs.

  1. 13 is a magic number. You already used the correct single-quote character representation elsewhere. Use that.

  2. Delays of more than 10ms are going to interfere with both serial ports. If you have accepting serial input you should not be using delay like this.

    relaytime = -10; //The time it takes for the relay to execute again (in seconds)

The comment is wrong. By setting it to -10 and making it count up to 10 that seems like 20 seconds to me.

Then I look at the definition for relaytime... It's a float. Don't use floats for counting. Adding a small float to a large float doesn't always give you the answer you expect. In the worst case a+b=a, when b is very much smaller than a.

(more on your timing errors later...)

  1. Now we get to the problem you actually complained about. Printing the pH. But what is the problem? Can you show exactly what was received by myserial, what was converted to float and what was saved to the file?

  2. You are adding data to the phdata array and then incrementing the index. But you never check if the index has walked off the end of the array. And you never reset the index back to zero after printing it. So you will definitely be writing into memory you don't own. That will always make your program behave in unexpected ways. The absolute best outcome for this is it simply stops working.

Because the array sizes are magic numbers, if you need to fix this problem you would have to put that same number in many places in the code. It would be best to have a constant like MAX_ARRAY_COUNT so you can initialize the arrays to that size and always check against that size before writing to the array.

  delay(1);

sensortiming = sensortiming+1;
  relaytime = relaytime+0.001;

This is a very fragile way to do timing. delay(1) does delay pretty much exactly 1 millisecond. But the rest of your code might have taken 2 or 3 or 3000 milliseconds. So your timing is always going to be slow.

There's two (three) ways to fix this:
A) Record millis() each time you get to this point and don't go past this point until the required milliseconds have passed since the previous time you recorded millis(). This method is good for PID and other applications that require precise timing and all the different sensor readings must be taken at the same instant.

If the time is 100ms per loop then this method is incompatible with serial inputs, for the reasons given above.

B) Let loop() run as fast as possible. Each different sensor can take a reading on its own schedule. Good for reading analog sensors multiple times and filtering the result while requesting data from digital sensors at a different period.

Optional: C) You can modify "A" so it will "catch up" with missed readings. If the SD card took 100ms to save occasionally but you need readings every 50ms, you can just take two readings close together to catch up on the one that was missed while the SD card was being slow.

  1. Then (further back in the code) you test if sensortiming is exactly equal to 1000. (Another magic number.) But what if you decided you wanted to make it faster for testing and added 3 at the bottom of the loop? Then it won't ever get to exactly 1000. Always make that kind of test a >= test, just in case you missed the exact moment.

MorganS:
0. Your array sizes are magic numbers. See #7 below for more.

The array sizes are arbitrary, I'll give you that, but that doesn't really matter because this is just a proof of concept. From my knowledge the sizes don't actually change a lot since I only need 15 things in the array.

MorganS:

  1. It will be a good idea to begin the SD card before attempting to delete any files on it.

Alright, but it's still not an actual issue we're having.

MorganS:
2. I've never seen readStringUntil() used inside serialEvent() I'm surprised that it works at all.

readStringUntil() is a blunt tool. serialEvent() isn't much sharper. They are best treated as "seemed like a good idea at the time" and should not be used in anything except small demonstrations.

You have the right idea in the myserial section later down in loop(). Use that method for both serial inputs.

  1. 13 is a magic number. You already used the correct single-quote character representation elsewhere. Use that.

This code is originally from a company called Atlas Scientific, readStringUntil() and serialEvent() work just fine so I see no reason to change it. Also, 13 isn't a magic number, it's the exact byte size we're expecting from the sensor.

MorganS:
4. Delays of more than 10ms are going to interfere with both serial ports. If you have accepting serial input you should not be using delay like this.

Luckily, we never have a delay of over 10 ms.

MorganS:
The comment is wrong. By setting it to -10 and making it count up to 10 that seems like 20 seconds to me.

Then I look at the definition for relaytime... It's a float. Don't use floats for counting. Adding a small float to a large float doesn't always give you the answer you expect. In the worst case a+b=a, when b is very much smaller than a.

(more on your timing errors later...)

The comment is wrong but again, doesn't matter. The reason I use a float instead of an integer is because if I use an integer and I try to set it back multiple hours then we have an over flow error and that's the last thing we want.

MorganS:
6. Now we get to the problem you actually complained about. Printing the pH. But what is the problem? Can you show exactly what was received by myserial, what was converted to float and what was saved to the file?

Like I said, the problem is that the string we're getting from the sensor is being rounded for some reason. mySerial receives a string (not a float or value) from the sensor and before I even convert it to a value it's rounded to seemingly no reason.

MorganS:
This is a very fragile way to do timing. delay(1) does delay pretty much exactly 1 millisecond. But the rest of your code might have taken 2 or 3 or 3000 milliseconds. So your timing is always going to be slow.

This was true in an older version of the code when we would just write the data when it came in, we realized this and switched it to this method, now everything works fine so the loop obviously doesn't take more than a millisecond to complete. That being said I would love a different way of doing this. The methods you provide however seem to need info from the sensors that I don't have and, once again, everything timing wise is pretty much fine.

MorganS:
9. Then (further back in the code) you test if sensortiming is exactly equal to 1000. (Another magic number.) But what if you decided you wanted to make it faster for testing and added 3 at the bottom of the loop? Then it won't ever get to exactly 1000. Always make that kind of test a >= test, just in case you missed the exact moment.

1000 is not a "magic number", it's the time in milliseconds I want it to go off at. Again though, this does work.

Stockbridge_InvenTeam:
This code is originally from a company called Atlas Scientific, readStringUntil() and serialEvent() work just fine so I see no reason to change it.

There were no names in the code you posted. You claimed it is all yours.

Stockbridge_InvenTeam:
Luckily, we never have a delay of over 10 ms.

Except for the three delay(1000)'s in a row.

Stockbridge_InvenTeam:
The comment is wrong but again, doesn't matter. The reason I use a float instead of an integer is because if I use an integer and I try to set it back multiple hours then we have an over flow error and that's the last thing we want.

If the comment is wrong it shouldn't be there. Misleading comments are worse than no comments.

We need to explore this "set it back" issue some more. What do you want it to do? You can use negative numbers with unsigned integers for timing purposes.

Stockbridge_InvenTeam:
Like I said, the problem is that the string we're getting from the sensor is being rounded for some reason. mySerial receives a string (not a float or value) from the sensor and before I even convert it to a value it's rounded to seemingly no reason.

Print the string. Print the converted value. Show us precise examples of what errors you are seeing.

Stockbridge_InvenTeam:
This was true in an older version of the code when we would just write the data when it came in, we realized this and switched it to this method, now everything works fine so the loop obviously doesn't take more than a millisecond to complete. That being said I would love a different way of doing this. The methods you provide however seem to need info from the sensors that I don't have and, once again, everything timing wise is pretty much fine.

It is not obvious at all that your code takes less than a millisecond. It can take 3000 milliseconds if it follows one code path. readStringUntil() will take up to 1000 milliseconds unless you change its default timeout.

Stockbridge_InvenTeam:
1000 is not a "magic number", it's the time in milliseconds I want it to go off at. Again though, this does work.

The Wikipedia entry on magic numbers s rather long and difficult to read. Here's a quicker explanation: CodeQL query help for C and C++ — CodeQL query help documentation

That explanation goes more directly to my point 0 and point 7. Yes, arbitrary sizes are just fine but you must be sure that you use the same size everywhere and make sure you don't run off the end of the array to damage memory you don't own.