SOS Hard Fault every once and a while, reboots

I have some pretty basic code which lets me control my lights and fan from my watch, but every once and a while (sometimes its fine for hours, sometimes its only a few minutes) It reboots, turning off my fan/lights. Before that happens I get the SOS one blink SOS red light which signifies a hard fault.
I’m pretty much teaching myself as I go along, so I do not know much besides the very basics, but most other threats said they need to see the code to diagnose the issue, so hopefully you can help me.
Heres my code:

int Fan = D3;
int Brightlight = D4;
int Desklight = D5;


void setup()
{



pinMode(Brightlight, OUTPUT);
pinMode(Fan, OUTPUT);
pinMode(Desklight, OUTPUT);


Spark.function("fanIO",fantoggle);
Spark.function("blIO",bltoggle);
Spark.function("dlIO",dltoggle);



}








int fantoggle(String command) {

if (command=="on") {
    
    digitalWrite(Fan,HIGH);
    return 1;
    Spark.publish("Fan On");
}
else if (command=="off") {
    
    digitalWrite(Fan,LOW);
    return 0;
    Spark.publish("Fan Off");
}

}

int bltoggle(String command) {

if (command=="on") {
    
    digitalWrite(Brightlight,HIGH);
    return 1;
    Spark.publish("BrightLight On");
}
else if (command=="off") {
    
    digitalWrite(Brightlight,LOW);
    return 0;
    Spark.publish("BrightLight Off");
}

}

int dltoggle(String command) {

if (command=="on") {
    
    digitalWrite(Desklight,HIGH);
    return 1;
    Spark.publish("DeskLight On");
}
else if (command=="off") {
    
    digitalWrite(Desklight,LOW);
    return 0;
    Spark.publish("DeskLight Off");
}

}

I have to confess that I don’t see any immediate reason for this behaviour, so I’m just going to guess a bit :wink:

If you are running a Core (rather than a Photon) this behaviour might pop up more frequently due to the smaller RAM.

It might be (very vague suspission) that your statements like if (command=="on") may repeatedly cause a new instance of a String("on") to be created an allocated in the heap area, which eventually causes the hard fault due to lack of new space for another instance.
To test this suspission and get around it, if it actually is the cause, you could try if (command.equals("on")) instead (there is an explicit overload String::equals(const char*) that definetly doesn’t create a String("on") object).

BTW: Your Spark.publish() never will be executed since the return statement preceeding it will leave the function just before it.

The only thing I see is you do not have a loop() function.

Maybe the compiler is smart enough to add the function, but try putting that in there (and making the changes @ScruffR cruff mentioned) like this:

int Fan = D3;
int Brightlight = D4;
int Desklight = D5;

void setup()
{
  pinMode(Brightlight, OUTPUT);
  pinMode(Fan, OUTPUT);
  pinMode(Desklight, OUTPUT);
  Spark.function("fanIO", fantoggle);
  Spark.function("blIO", bltoggle);
  Spark.function("dlIO", dltoggle);
}

void loop()
{
  
}

int fantoggle(String command)
{
  if (command == "on") 
  {
    digitalWrite(Fan, HIGH);
    Spark.publish("Fan On");
    return 1;
  }
  else if (command == "off")
  {
    digitalWrite(Fan, LOW);
    Spark.publish("Fan Off");
    return 0;
  }
}

int bltoggle(String command)
{
  if (command == "on") 
  {
    digitalWrite(Brightlight, HIGH);
    Spark.publish("BrightLight On");
    return 1;
  }
  else if (command == "off") 
  {
    digitalWrite(Brightlight, LOW);
    Spark.publish("BrightLight Off");
    return 0;
  }
}

int dltoggle(String command) 
{
  if (command == "on") 
  {
    digitalWrite(Desklight, HIGH);
    Spark.publish("DeskLight On");
    return 1;
  }
  else if (command == "off") 
  {
    digitalWrite(Desklight, LOW);
    Spark.publish("DeskLight Off");
    return 0;
  }
}
1 Like

@BulldogLowell, nice rewrite and @ScruffR good catch on the the code not getting to publishes due to returns. @chtapodi, I highly recommend looking at code from other members in the ProjectShare area, as well as, the great projects in hackster.io. You can learn a lot about programming for the Core and Photon doing that. :smiley:

Thank you for your suggestions. I replaced that and now I’m going to leave it for a while and see if it still reboots.
I’ll get back with an update.
Thank you

2 Likes

I removed the loop function just before posting this because I thought that it could be the issue due to it having nothing in it.
I put it back in
Thank you for your help, hopefully it’ll start working now.

Unfortunately I am still having the same issue.
Do you think the problem could be on the watch side of things?
I have included the code I lobbed together from other peoples projects to make a working watchapp.

var UI = require('ui');
var ajax = require('ajax');
//var Vector2 = require('vector2');
//var Accel = require('ui/accel');
var Vibe = require('ui/vibe');
//var Settings = require('settings');

// -------- Global Variables -------- ///
var particleAPIURL = 'https://api.particle.io/v1/';
var particleDeviceID = '1e0029000547343337373738';
var particleAPIKey = '5e6258fe38fd74782b03e54885850847c40517d7';
var fanIO = 'off';
var dlIO = 'off';
var blIO ='off';

var cardSplashScreen = new UI.Card();
cardSplashScreen.title('Room');
cardSplashScreen.subtitle('Loading...');
cardSplashScreen.body('');
cardSplashScreen.show();

// Load the API information
ajax(
  {
url: particleAPIURL + 'devices/' + particleDeviceID + '/?access_token=' + particleAPIKey,
type:'json'
  },
function(data) {
// Notify the user
Vibe.vibrate('short');

console.log("Got data: " + JSON.stringify(data));

// Check if the device is online
if(data.connected === true)
{
  // Construct a Menu to show to the user
  console.log("Device is online. Show the menu");
  var menuControl = new UI.Menu({
    title: 'Controls',
    sections: [{
      title: 'Control',
      items: [{
        title: 'Desklight'
        
      },{
        title: 'Brighlight'
      },{title: 'Fan'}]
    }]
  });

  // Add an action for the 'select' button (middle button)
  menuControl.on('select', function(e) {
    console.log('Item number ' + e.itemIndex + ' was pressed!!');
    
    if(e.itemIndex === 2)
    {
				if(fanIO == 'off') {
				fanIO = 'on';
       ajax(
      {
        url: particleAPIURL + 'devices/' + particleDeviceID + '/fanIO',
        method: 'post',
        data: {access_token: particleAPIKey, args: fanIO}
      });
				}
				else {
				fanIO = 'off';
       ajax(
      {
        url: particleAPIURL + 'devices/' + particleDeviceID + '/fanIO',
        method: 'post',
        data: {access_token: particleAPIKey, args: fanIO}
      });
				}
      
    }
    else if (e.itemIndex === 0)
    
				if(dlIO == 'off') {
				dlIO = 'on';
       ajax(
      {
        url: particleAPIURL + 'devices/' + particleDeviceID + '/dlIO',
        method: 'post',
        data: {access_token: particleAPIKey, args: dlIO}
      });
				}
				else {
				dlIO = 'off';
       ajax(
      {
        url: particleAPIURL + 'devices/' + particleDeviceID + '/dlIO',
        method: 'post',
        data: {access_token: particleAPIKey, args: dlIO}
      });
				}
      
    
    else if (e.itemIndex === 1)
    
				if(blIO == 'off') {
				blIO = 'on';
       ajax(
      {
        url: particleAPIURL + 'devices/' + particleDeviceID + '/blIO',
        method: 'post',
        data: {access_token: particleAPIKey, args: blIO}
      });
				}
				else {
				blIO = 'off';
       ajax(
      {
        url: particleAPIURL + 'devices/' + particleDeviceID + '/blIO',
        method: 'post',
        data: {access_token: particleAPIKey, args: blIO}
      });
				}
      
    
    
    
  });
  

  menuControl.show();
  cardSplashScreen.hide();
}
else
{
  // Failure (device is offline)
  Vibe.vibrate('double');
  console.log("Device is offline. Show an error message");
  cardSplashScreen.subtitle('');
  cardSplashScreen.body('Device is offline.');
}

  },
  function(error) {
// Failure (bad response from the API)
Vibe.vibrate('double');
console.log('Download failed: ' + JSON.stringify(error));
cardSplashScreen.subtitle('');
cardSplashScreen.body('Unable to communicate with the Particle API');
  }
);

Thank you for the help.

Sorry for not spotting it the first time, but you have to finish your functions with a return statement that actually returns an int, otherwise you will mess up your stack.

So if none of your if condition catches (maybe due to a faulty remote call) you reach the end of the function without a return statement and get a hard fault some time after.

I usually don’t use multiple return statements but use a retVal variable that gets passed back in the final line of code return retVal;. This also prevents problems as above that desired code does not get executed due to premature exit.

Shouldn’t that be spotted by a compiler warning? Afaik the WebIDE/cloud compiler builds with -Wall and if it doesn’t it probably should. Also I’m a big fan of multiple returns as they can help to keep the control flow depth to a minimum
and especially when dealing with error conditions it also helps to keep things very clean for example parsing simple commands (semi pseudocode untested)
`

 int read_command(Socket& sock) {
     size_t length = read(&buf[0], buf_length);
     if(length < expected_length){
          // handle error
          return -1;
     }
    // here we can safely access buf[0] to buf[expected_length-1]
    // without increased control flow depth

    return length;
}

`
so yeah just a little rant as I think the argument of “return retVal” preventing missing returns is mood as they are easy to find for the compiler. Also I feel like it is in fact more error prone the retVal can easily be overwritten and one needs to awkwardly
get out of error conditions and to the end of the function.

Yes, but when there is no error the warnings are not shown on Particle Build.
Particle Build used to have -Wall but that was not taken up with too much enthusiasm, so it got removed (on “public” demand).


I’ll take the return hint as a matter of personal preference :wink:
e.g. http://bytes.com/topic/c/answers/617841-multiple-returns-functions
Using a few early returns for your error conditions is OK, scattering loads all over the place and in all sorts of nesting levels does not help readability.

And

I was not refering to missing return but to an earlier comment of mine which pointed out a problem of code flow in the OP’s code :sunglasses:

int fantoggle(String command) {
  if (command=="on") {
    digitalWrite(Fan,HIGH);
    return 1;
    Spark.publish("Fan On");
  }
  else if (command=="off") {
    digitalWrite(Fan,LOW);
    return 0;
    Spark.publish("Fan Off");
  }
}

Hence the wording

Interesting is there a way to get all warning e.g. when using particle-cli? I don’t use C/C++ that much but my friends that do almost always use -Wall -Werror and I’ve always been using at least -Wall.

AFAIK, no, not on the cloud builds (yet???).
But local build gives you all the options needed.

One (stupid) workaround is, to add an erronous statement to trip the build, then you get all the warnings too :stuck_out_tongue_winking_eye:

Folks, let’s take the compiler flags to another topic. In the original OP’s code, there is no fall-through return statement as was pointed out. So regardless of how a value is returned it must be returned nonetheless. Adding a return -1; after each else if would take care of that. So, for example:

int bltoggle(String command)
{
  if (command == "on") 
  {
    digitalWrite(Brightlight, HIGH);
    Spark.publish("BrightLight On");
    return 1;
  }
  else if (command == "off") 
  {
    digitalWrite(Brightlight, LOW);
    Spark.publish("BrightLight Off");
    return 0;
  }
  return -1;    // If both if statements fail, a -1 is returned
}
3 Likes

Not related to OPs particular situation, but related to the same subject:

Had all of my P1s (currently half a dozen) randomly crashing with hard fails. Sometimes they would sit for hours without problem, sometime they would SOS every few seconds. Because same code worked on Photon without issues I spent hours trying to chase power brown-out issues adding capacitors and instructors, trying different regulators etc to no avail. Also tried various software changes up to just empty setup/loop in auto mode and it would still crash though not as frequently.

Just for kicks I updated P1’s firmware via DFU interface to v0.4.4-rc.3 and the problem went away. Just like that. My understanding was that firmware updates are pushed over the air, but I suppose that’s not the case. Anyway, if anybody else is struggling with flaky P1’s this could be your solution.

@sparkly, updates will be pushed OTA once the new firmware is released but in a “controlled” fashion. The firmware will be updated if a user compiles code requiring an update to the system parts OR if a user specifically requests the update. The idea is not to force an update that could break a working environment. :smile:

I want to correct some minor misinformation here.

Not returning the type of object that a function is declared to return will not mess up your stack. It can (pronounced “will”) mess up the caller, if it relies on a particular return value (especially when that return value is a pointer.)

3 Likes

Hi everyone,
Thank you so much for all the help!
I’ve gone through and implemented a lot of the suggested changes as well as force updating the firmware, and I haven’t had it crash for at least 12 hours.Hopefully it remains functional.

1 Like