Trouble creating motion detector with arduino uno and MPU6050 accelerometer

I am BRAND new to this. I am trying to create essentially a motion detector with an accelerometer and an arduino uno so that the when the accelerometer is moved, an LED lights up. I have been successfully able to read data from the MPU6050 accelerometer.

I have even been able to sometimes get the program to store the last read value of AcX and subtract the current value from it to determine if the accelerometer is moving. However, half of the time the program returns zeros, instead of values created by subtracting the first reading from the second.

Can anyone take a look at this code and tell me why it is so fragile? Eventually, I want to be able to check if all three variables (x, y, z) have changed.

This is the entire text of the program. Everything up to the delay(1000) is copied directly from a tutorial in reading the accelerometer. The code below that, beginning with the lastRead function is mine. The error(s) are somewhere in there:

#include<Wire.h>

const int MPU_addr=0x68;  // I2C address of the MPU-6050
int16_t AcX, AcY,AcZ;

int buttonInput = 7;
int lightOutput = 8;

 int x=AcX;
 int y=AcY;
 int z=AcZ;


void setup(){
  Wire.begin();
  Wire.beginTransmission(MPU_addr);
  Wire.write(0x3B);  // PWR_MGMT_1 register
  Wire.write(0);     // set to zero (wakes up the MPU-6050)
  Wire.endTransmission(true);
  Serial.begin(9600);

  pinMode(lightOutput, OUTPUT);
  digitalWrite(buttonInput, LOW);
}


void loop(){
  Wire.beginTransmission(MPU_addr);
  Wire.write(0x3B);                    // starting with register 0x3B (ACCEL_XOUT_H)
  Wire.endTransmission(false);
  Wire.requestFrom(MPU_addr,14,true);  // request a total of 14 registers
  AcX=Wire.read()<<8|Wire.read();      // 0x3B (ACCEL_XOUT_H) & 0x3C (ACCEL_XOUT_L)     
  AcY=Wire.read()<<8|Wire.read();    // 0x3D (ACCEL_YOUT_H) & 0x3E (ACCEL_YOUT_L)
  AcZ=Wire.read()<<8|Wire.read();    // 0x3F (ACCEL_ZOUT_H) & 0x40 (ACCEL_ZOUT_L)
  
  Serial.print("AcX = "); Serial.print(AcX);
  Serial.print(" | AcY = "); Serial.print(AcY);
  Serial.print(" | AcZ = "); Serial.println(AcZ);
  
  
  delay(1000);


//this is where my problems probably begin
  
int16_t  lastRead(x);
{
  x=AcX;
  Serial.print("x = "); Serial.println(x);
}


int16_t newX;

int16_t  compareRead(newX);
{
   newX = (lastRead-AcX);
   Serial.print("newX = "); Serial.println(newX);
}


//this part of the code works, if I can get newX to equal the first reading minus the second reading, instead of the current reading minus the current reading, which equals 0.

if(newX > 1000){
  digitalWrite(lightOutput, HIGH);
  delay(1000);
   digitalWrite(lightOutput, LOW);
}
if(newX < -1000){
  digitalWrite(lightOutput, HIGH);
  delay(1000);
  digitalWrite(lightOutput, LOW);
}
else{
return(0);
}
}

It looks like you've used this as an example: Arduino Playground - MPU-6050

You problems start quite early. You are interested in the value of x (which you derive from AcX) but you set x even before AcX has been given a value and never set it again. Therefore x is 0 and will always be 0.

int x=AcX;
 int y=AcY;
 int z=AcZ;
 . . .
void setup(){

This looks remarkably like an attempt at a function definition, but you have nested it in the function loop() which is not allowed.

int16_t newX;

int16_t  compareRead(newX);
{
   newX = (lastRead-AcX);
   Serial.print("newX = "); Serial.println(newX);
}

If you are attempting to compare the current value of something with its previous value, you have declare the previous value as a global variable, so it [that previous value] is not lost when the function loop() is called again.

For your further experiments, it might be simpler to define a global variable

int16_t AcXPrevious ;

At the very end of the loop(), set AcXPrevious to the current value of AcX.

In the middle of the loop() you can do a compare of AcX and AcXPrevious and see if there is a difference, and act accordingly.