attach interrupt ISR

Using Arduino Programming Questions
1

Error message saying button1_ISR is not declared on this scope?

Basically controlling a fan here, I am trying to make button1_ISR into an interrupt loop so that when a button is pushed the motor speed will increase by 5 between 180 and 230. Am I doing something stupid?
Seems pretty basic to me I must be doing something wrong.

int motor = 6;
const int button1Pin = 4;
const int button2Pin = 3;
const int button3Pin = 2;
const int button4Pin = 1;
int motorStart = 220;
int motorSpeed = 180;
int onMins = 0;
int offHours = 0;
volatile int button1State = 0;

void setup() {
Serial.begin(9600);
pinMode (button1Pin, INPUT_PULLUP);
pinMode (button2Pin, INPUT_PULLUP);
pinMode (button3Pin, INPUT_PULLUP);
attachInterrupt(digitalPinToInterrupt(3),button1_ISR, CHANGE);
}

void loop() {

int press1 = digitalRead(5);
if (press1 == LOW) analogWrite (motor, motorStart);
delay (200);
analogWrite (motor, motorSpeed);
delay (500);

void button1_ISR() {
delay (100);
motorSpeed = motorSpeed+5;

if (motorSpeed = 230)
{
motorSpeed = 180;
}

Serial.print( " motorSpeed: ");
Serial.println(motorSpeed);

}

2

Missing a closing brace on loop so the ISR is defined inside loop. Can't define a function inside another function.

You also shouldn't be trying to print to serial from inside the ISR as that can deadlock the board.

3

pwatsoon:
Am I doing something stupid?

Don't use delay in an ISR.
Don't use Serial.print/write in an ISR (not so deadly, but just don't).
Don't use ISR's for bouncing keys.
Make all variables shared between ISR and main code volatile.
Protect all accesses to multi-byte volatile entities by making them atomic.

4

I am trying to make button1_ISR into an interrupt loop so that when a button is pushed the motor speed will increase by 5

Unless you are doing it for interest then why use an interrupt ? Just read the input in loop() and act on it when it changes.

By the way this

  if (motorSpeed = 230)

sets motorSpeed to 230 rather than testing its value.

const int button1Pin = 4;
const int button2Pin = 3;
const int button3Pin = 2;
const int button4Pin = 1;

Gives meaningless names to pins, including pin 1 which is used by Serial. Some of them are later set to INPUT_PULLUP but you never read any of them. Instead you read the anonymous pin 5. What is that all about ?