Particle.subscribe() corrupts program

Troubleshooting
1

I’ve got a lovely program that watches some buttons and opens some relays on a Photon and relay shield. However, once I add the Particle.subscribe() line, two of the buttons stop working, and the third runs actions of the other buttons.
This is my second Photon. I got my first Photon to play with IFTTT integration, and it still runs like a champ.

2

Hi @nstanleyiv

Because Particle.subscribe() hands you a pointer to internal data structures used by the cloud connection, how you handle that received data makes a big difference. For instance if you try to republish or otherwise use the received strings without copying them, bad things can happen in memory on your device. This might sound like a bug but letting you copy the data when needed is a lot better than always copying the data and running out of memory–it is just a trade-off.

Can you share your code? If not the entire thing just the subscribe handler function?

3 
#include "application.h"

//Constants
#define POUR_TIME 5000
#define LIGHT_TIMEOUT 1000

//Renames
#define ON true
#define OFF false
#define STATE_PUSH true
#define STATE_RELEASE false
#define BUTTON_PRESS false
#define BUTTON_RELEASE true
#define ONE_SHOT true

//Output Pins
#define SPOUT_RELAY D3
#define FRIDGE_RELAY D4
#define UPPER_LIGHTS_RELAY D5
#define LOWER_LIGHTS_RELAY D6
#define LIGHTS_LED D0
#define POUR_LED D1
//Input Pins
#define PRESSURE_PIN A1
#define LIGHTS_BUTTON D2
#define POUR_BUTTON D7

//State Tracking
bool semaPour = OFF;
bool LightState = OFF;
bool SavedLightState = OFF;
bool GlassPresent = false;
bool LightRequest = false;
bool PourRequest = false;
bool PourCompleted = false;
bool PourStarted = false;

//Pour timer
Timer PourTimer(POUR_TIME, StopPour, ONE_SHOT);

/*
 * DoPour
 * If there is a glass present, turn the lights on and pour the beer
 *
 */
void DoPour(const char *e, const char *arg)
{
    if(!GlassPresent)
        return;
    SavedLightState = LightState;
    SetLights(ON);
    //Turn on the relays and start the timer
    digitalWrite(SPOUT_RELAY, HIGH);
    digitalWrite(FRIDGE_RELAY, HIGH);
    digitalWrite(POUR_LED, HIGH);
    PourTimer.start();
}//end DoPour()

/*
 * StopPour
 * Turn off the pour relays, turn lights off after glass taken
 *
 */
void StopPour()
{
    //Stop pouring!
    digitalWrite(SPOUT_RELAY, LOW);
    digitalWrite(FRIDGE_RELAY, LOW);
    digitalWrite(POUR_LED, LOW);
    SetLights(OFF);
}//end StopPour()

/*
 * SetLights
 * Turn light relays on or off
 *
 */
void SetLights(bool b)
{
    //Turn on
    if(b)
    {
        digitalWrite(UPPER_LIGHTS_RELAY, HIGH);
        digitalWrite(LOWER_LIGHTS_RELAY, HIGH);
        digitalWrite(LIGHTS_LED, HIGH);
        LightState = ON;
    }
    else //turn off
    {
        digitalWrite(UPPER_LIGHTS_RELAY, LOW);
        digitalWrite(LOWER_LIGHTS_RELAY, LOW);
        digitalWrite(LIGHTS_LED, LOW);
        LightState = OFF;
    }
}//end SetLights()

/*
 * LightButtonPress
 * Toggle lights, based on internal tracking
 *
 */
void LightButtonPress()
{
    LightRequest = true;
}

void PourButtonPress()
{
    PourRequest = true;
}

void PressureApplied()
{
    if(digitalRead(PRESSURE_PIN))
    {
        GlassPresent = false;
    }
    else
    {
        GlassPresent = true;
    }
}
/*
 * setup
 * Default function for initialization
 *
 */
void setup() 
{    //Pin setup
    //Buttons
    pinMode(LIGHTS_BUTTON, INPUT_PULLUP);
    pinMode(POUR_BUTTON, INPUT_PULLUP);
    pinMode(PRESSURE_PIN, INPUT_PULLUP);
    //Relays
    pinMode(SPOUT_RELAY, OUTPUT);
    pinMode(FRIDGE_RELAY, OUTPUT);
    pinMode(UPPER_LIGHTS_RELAY, OUTPUT);
    pinMode(LOWER_LIGHTS_RELAY, OUTPUT);
    //LEDs
    pinMode(LIGHTS_LED, OUTPUT);
    pinMode(POUR_LED, OUTPUT);
    
    //Interupts
    attachInterrupt(LIGHTS_BUTTON, LightButtonPress, FALLING);
    attachInterrupt(POUR_BUTTON, PourButtonPress, FALLING);
    attachInterrupt(PRESSURE_PIN, PressureApplied, CHANGE);
    
    //All off
    digitalWrite(SPOUT_RELAY, LOW);
    digitalWrite(FRIDGE_RELAY, LOW);
    digitalWrite(UPPER_LIGHTS_RELAY, LOW);
    digitalWrite(LOWER_LIGHTS_RELAY, LOW);
    digitalWrite(LIGHTS_LED, OFF);
    digitalWrite(POUR_LED, OFF);
    
    //Cloud function
    //Spark.subscribe("Beer", DoPour);
}//end setup()

/*
 * loop
 *
 */
void loop() 
{
    if(LightRequest)
    {
        SetLights(!LightState);
        LightRequest = false;
    }
    if(PourRequest)
    {
        DoPour(NULL,NULL);
        PourRequest = false;
    }
    if(!GlassPresent)
    {
        StopPour();
    }
    //delay(200);
}//end loop()
4

The DoPour function doesn’t do anything with received data

5

@nstanleyiv, I can’t see anything specifically wrong with your subscribe though I can make some suggestions on the code overall:

  1. You have no debounce on your interrupt driven buttons which may cause multiple interrupts. You may want to consider using the clickButton library which creates polled buttons but does all of the heavy lifting. It’s one of my faves.

  2. In your button ISRs you are setting the value of non-volatile globals, possibly multiple times due to the lack of debouncing. All variables that you expect to change in a Timer or an ISR should be made of type volatile.

The above conditions could reap havoc so you can retest subscribe once you’ve addressed these. :wink:

6

Also, you probably want to subscribe using MY_DEVICES. Otherwise, if anyone in the world publishes a public event whose name begins with “Beer" your function will be triggered. I briefly tested it, and I’m getting at least one per second in the public feed!

Particle.subscribe("Beer", DoPour, MY_DEVICES);
2 Likes
7

I like the point @rickkas7 made about subscribing to frequent event!

One other thing I noticed is that Yyou have one pin (A1) which is set to interrupt on CHANGE but has pinMode set to INPUT_PULLUP. I thought there was an assumption somewhere that pull-up/down state should not cause an interrupt and there was system code that got in the way of using of interrupts the way you are.

Does anyone else (@peekay123 or @ScruffR in particular) remember this?

Pin D7 (POUR_BUTTON) is also a JTAG pin so things will happen before you code is run on that pin.

Maybe external pull-ups would be a better idea anyway since they are there from time zero.

2 Likes
8

@bko, I’ve looked at the code and nothing suggests a change of mode for the pin when attachInterrupt() is used though I can’t say it was an in-depth analysis.

Having a stream of subscribes is never good and neither is generic event names IMO :wink:

@nstanleyiv should take our suggestions, make some changes and let us know how things stand.

1 Like
9

Thanks all for the code review! This is my first “large” Photon program, and your feedback will be invaluable going forward… I’m betting the controller was just getting bogged down from Beer events. I’ve made it private, and will let you know how it goes once I get home from work

1 Like
10

The thing I think you remember there was in connection with sleep and wake-on-pin.
The RISING and FALLING versions do override any pinMode() set prior to System.sleep().

But for general CHANGE interrupts (as for the others too) you definetly need to make sure your pin never floats.

1 Like