PID problem transmitter.sendUnit(0, true);

I am using the beginner PID provided in the Arduino Library and the Relay Output example with modifications.

I have commented out transmitter.sendUnit(0, true);and transmitter.sendUnit(0, false); because it stops the if/else statement from switching to the else statements when the output is rising/high

I suspect this is because of the 257ms transmit time set up

NewRemoteTransmitter transmitter(4586050, 9, 257, 1);

or perhaps an oversight with curly braces but i am not sure.

Any pointers would be appreciated.

Thanks
Leo

#include <EEPROMEx.h>
#include <SHT1x.h>
#include <PID_v1.h>
#include <NewRemoteTransmitter.h>

NewRemoteTransmitter transmitter(4586050, 9, 257, 1);

#define RelayPin 8
#define dataPin  12
#define clockPin 13
SHT1x sht1x(dataPin, clockPin);



double tempSetpoint, tempInput, tempOutput;
PID tempPID(&tempInput, &tempOutput, &tempSetpoint,40, 5, 1, REVERSE);

int WindowSize = 500;
unsigned long windowStartTime;
void setup()
{
  windowStartTime = millis();

  //initialize the variables we're linked to
  tempSetpoint = 22;

  //tell the PID to range between 0 and the full window size
  tempPID.SetOutputLimits(0, WindowSize);

 
  tempPID.SetMode(AUTOMATIC);
  Serial.begin(9600);
}

void loop(){ 

  {
    double temp_c;
    tempInput = temp_c = sht1x.readTemperatureC();
    tempPID.Compute();

    /************************************************
     * turn the output pin on/off based on pid output
     ************************************************/
    if(millis() - windowStartTime>WindowSize)
    { //time to shift the Relay Window
      windowStartTime += WindowSize;
    }
    if(tempOutput < millis() - windowStartTime){
      //transmitter.sendUnit(0,false);
      Serial.print("Fans, Off ");
      digitalWrite(RelayPin,LOW); 
      
    }
    else{
      //transmitter.sendUnit(0,true);
      Serial.print("Fans, On ");
      digitalWrite(RelayPin,HIGH);
     
    }



    Serial.print(" Temprature ");
    Serial.print(temp_c     );
    Serial.print("  TempSetpoint  ");
    Serial.print(  tempSetpoint , DEC );
    Serial.print("  tempOutput  ");
    Serial.println(tempOutput);
  }


}

Any pointers would be appreciated.

char *ptr = "You need to do a better job describing what the problem is, not what you have tried to do to solve it";

I am running the PID and it works as expected.

I am printing "tempOutput" from the PID to the serial so I can monitor the changes and as "tempOutput" rises or falls the if/else statement is executed correctly, if the temperature is constantly under the set point the output will hold on 0.00 and if its constantly over it will hold on 500.00

int WindowSize = 500;

When the "tempInput" is below the "tempSetpoint" or falling it executes everything under the if

 if(tempOutput < millis() - windowStartTime){
      //transmitter.sendUnit(0,false);
      Serial.print("Fans, Off ");
      digitalWrite(RelayPin,LOW); 
      
    }

and when the "tempInput" is above the "tempSetpoint" or rising it executes everything under the else

 else{
      //transmitter.sendUnit(0,true);
      Serial.print("Fans, On ");
      digitalWrite(RelayPin,HIGH);
     
    }

This works fine but I would also like to add the lines transmitter.sendUnit(0,true); and transmitter.sendUnit(0,false);.
When I add these lines to the code the "tempOutput" continues to rise and fall the PID reacts to temperature changes but only the if statement will run.
Even if the temperature is kept above "tempSetpoint" and the output holds at 500 I the else statement wont run

double tempSetpoint, tempInput, tempOutput;

Are these tempXXX as in temperatureXXX or as in temporaryXXX?

If tempOutput is a temperature, then comparing it to time doesn't make sense. If tempOutput is temporary, then making it a global variable doesn't make sense. If tempOutput is neither a temperature or temporary, then perhaps it should be renamed to George.

    double temp_c;
    tempInput = temp_c = sht1x.readTemperatureC();

What is temp_c for? It's sole purpose seems to be to have something to print later. Move the print() call to just after the read, and get rid of temp_c.

A link to the NewRemoteTransmitter would be good.

tempratureXXX.

In future I want to add another PID controlling Humidity and will call it humPID and set it up as so.

double humSetpoint, humInput, humOutput;
PID humPID(&humInput, &humOutput, &humSetpoint,40, 5, 1, REVERSE);

I thought I should just begin by naming the PID that controls the temperature tempPID instead of having to rename it later when the extra PID's are added into the code. Sorry for the confusion.

Is this ok or should I name them like this.

double Setpoint1, Input1, Output1;
PID PID1(&Input1, &Output1, &Setpoint1,40, 5, 1, REVERSE);

temp_c is just there so I can see what the current temp is, I have removed this and moved the print() as suggested

fuzzillogic / 433mhzforarduino / wiki / Home — Bitbucket is the page that links to the library for NewRemoteTransmitter (https://bitbucket.org/fuzzillogic/433mhzforarduino/get/latest_stable.zip)

Thanks for the help

NewRemoteTransmitter.cpp (2.97 KB)

NewRemoteTransmitter.h (3.44 KB)

LightShow.ino (1.04 KB)

My code with suggested changes.
tempPID has been renamed PID1 and all the relevant setpoints etc have been renamed from tempXXX to XXX1

#include <EEPROMEx.h>
#include <SHT1x.h>
#include <PID_v1.h>
#include <NewRemoteTransmitter.h>

NewRemoteTransmitter transmitter(4586050, 9, 257, 1);

#define RelayPin 8
#define dataPin  12
#define clockPin 13
SHT1x sht1x(dataPin, clockPin);



double Setpoint1, Input1, Output1;
PID PID1(&Input1, &Output1, &Setpoint1,40, 5, 1, REVERSE);

int WindowSize = 500;
unsigned long windowStartTime;
void setup()
{
  windowStartTime = millis();

  //initialize the variables we're linked to
  Setpoint1 = 22;

  //tell the PID to range between 0 and the full window size
  PID1.SetOutputLimits(0, WindowSize);


  PID1.SetMode(AUTOMATIC);
  Serial.begin(9600);
}

void loop(){ 

  {
    double temp_c;
    Input1 = sht1x.readTemperatureC();
    Serial.print(" Input1 ");
    Serial.print(  Input1     );
    Serial.print("  Setpoint1  ");
    Serial.print(  Setpoint1  );
    Serial.print("  Output1  ");
    Serial.println(Output1);
    PID1.Compute();

    /************************************************
     * turn the output pin on/off based on pid output
     ************************************************/
    if(millis() - windowStartTime>WindowSize)
    { //time to shift the Relay Window
      windowStartTime += WindowSize;
    }
    if(Output1 < millis() - windowStartTime){
      //transmitter.sendUnit(0,false);
      Serial.print("Fans, Off ");
      digitalWrite(RelayPin,LOW); 


    }
    else{
     // transmitter.sendUnit(0,true);
      Serial.print("Fans, On ");
      digitalWrite(RelayPin,HIGH);

    }




  }


}

And this is another reason you should read the stickies and all documentation
Apologies to PaulS for timewasting
attachinterrupt(); solved everything