Help me understand what happened

[rimai]

Hey Curt,

I have this in my setup()

Code: Select all

ReefAngel.OverheatShutoffPorts = B00000000;

I would assume this would do nothing in the case of overheat, since I have no port flagged to be turned off.
But somehow, when I got home today, I found my tank with all ports masked off.
Interestingly enough, I don't have *OverheatTempProbe assigned, but somehow it was using Temp3, because it was kind of sunny today and I saw sun on my Temp3 probe and the temperature got to above 100F. I used Temp3 for room temperture. Not that the room was 100F, but with the sun shinning on the probe, it increased the temperature locally on the probe itself.
I tested the same behavior again with a lighter, by heating temp3 above 100F and again the controller shut down all ports.
I went on the libraries and saw that overheat temp is a byte variable and has this code:

Code: Select all

// if overheat probe exceeds the temp
if ( *OverheatTempProbe >= InternalMemory.OverheatTemp_read() )
{
    LED.On();

Well, my red LED wasn't on.
The byte variable threw me off, because it can only go to 255, which would mean 25.5F
If I don't have *OverheatTempProbe assigned in the PDE, it does not have a initial value anywhere else.
I didn't see anywhere in the code where the pointer would get assigned a value.
Maybe the pointer is picking up Temp3 by coincidence?
So, I'm kind of totally lost and decided to just see if had anything to light up my head.
I'm working on a little surprise for tonight, if I can get it done... lol
So, I revisit this issue when I have more time if nothing comes to your mind.

[binder]

That is odd that happened to you. It defaults to using temp2 if nothing is assigned in the PDE file.
Inside the Init() function, there is this line:

Code: Select all

OverheatTempProbe = &Params.Temp2;

OverheatTempProbe should be an int pointer as defined in reefangel.h:

Code: Select all

int *OverheatTempProbe;

So in essence, it's just a pointer to the value stored in Params.Temp2 and then it's dereferenced to get the value to be compared to the overheat temp.

Then when the relays all getting shutoff is odd too. They shouldn't because of how the masking works. The negative for all 0's for the offmask is the normal way (which you already know). So this behavior is really odd.

I wonder if there was some mismatch with the libraries when you compiled things. I know that arduino has to be closed anytime you update the libraries or make changes otherwise you get undesired results. This will require some further investigation to see if it can be reproduced again outside of your code.

curt

[rimai]

Ok, after further investigation, I narrowed it down to an error on the array size.
Can you confirm?
On Globals.cpp:

Code: Select all

void ConvertNumToString(char* string, int num, byte decimal)
{
    char temptxt[3];

It has to be 5, right?
The issue was happening when any parameter reached 1000, which would overload the array.
I was actually using this on my PDE, which may have contributed even more:

Code: Select all

  char text[7];
  ConvertNumToString(text, ReefAngel.Params.Temp1, 10);
  strcat(text,"  ");

The reason I was adding the two additional spaces is to erase prior readings that were higher than the new one.
I was getting left over "0" showing "99.90" instead of "99.9" after it had displayed "100.0"
So, I think we should add the strcat to the ConvertNumToString too.
If you agree, I'll send another pull request with the fix.

[binder]

Ok, after further investigation, I narrowed it down to an error on the array size.
Can you confirm?
On Globals.cpp:

Code: Select all

void ConvertNumToString(char* string, int num, byte decimal)
{
    char temptxt[3];

It has to be 5, right?
The issue was happening when any parameter reached 1000, which would overload the array.

That is only used when we are computing the remainder. If you look at the function, there is only 1 place it is being written to.

Code: Select all

void ConvertNumToString(char* string, int num, byte decimal)
{
    char temptxt[3];
    int Temp = num;
    if (Temp==0xFFFF) Temp=0;
	itoa(Temp/decimal,string,10);
	if (decimal>1)
	{
		itoa(Temp%decimal,temptxt,10);  // written here
		strcat(string , ".");
		if (Temp%decimal<10 && decimal==100) strcat(string , "0");
		strcat(string , temptxt);  // copied over here
	}
}

That section is just taking the digits after the "decimal" place. The max decimal value that we use currently is 100 for the ph which gives us 2 decimal places. Based on that code, we are only using 2 of the 3 chars. So I don't think it's getting overloaded. If anything, there is an extra space/char that might need to be removed. I have to look more at how the arduino strcat function works.

I was actually using this on my PDE, which may have contributed even more:

Code: Select all

  char text[7];
  ConvertNumToString(text, ReefAngel.Params.Temp1, 10);
  strcat(text,"  ");

The reason I was adding the two additional spaces is to erase prior readings that were higher than the new one.
I was getting left over "0" showing "99.90" instead of "99.9" after it had displayed "100.0"
So, I think we should add the strcat to the ConvertNumToString too.

When we get to temps over 100 or ph over 10, there would be 5 chars being used in the string. The problem you/we are experiencing with the "ghosting" of the numbers (like with "99.90" after it being "100.0") is a problem with how the functions draw on the screen. They always start at the same position and draw over the spot. The 99.9 is shorter than 100.0, so the last space never gets cleared out. You adding an extra space at the end ensures that the string is the same length and the space is cleared.

Adding the extra space at the end for values less than 100 (or 10 for ph or 1000 if you just focus on the base value) would help ensure that this doesn't happen.

I don't think the buffer sizes need to be changed. Everything that I'm looking at makes sense to me so far and appears to be correct. I do want to keep thinking about it though as maybe I'm missing something too.

curt

[rimai]

Maybe it was my sketch that had wrong size then.
You got me thinking now if I had [5] or [7] as I typed on previous post.
[5] would definitely overload.
I'll test things out more when I go back home.

[binder]

ok. im curious to know what it is too.

curt

[rimai]

The issue here was my custom variable that was too short and was getting overloaded.
I changed it to a bigger array and it worked fine.

[binder]

Good to know. That's a note that we will have to keep in mind. Not sure how we can test or safeguard against that one.

curt