Any Glaring errors in this code? Will it work?

Yes, It compiles. I am working on a bot that will be controlled by jeenodes, but first I am trying to get it working non remote. The idea is to grab some variable pressure buttons from a couple of old game controllers, and I am hoping this code will work as I intend it to, but I do not have all the hardware present to fully test. The code is intended to control an H Bridge, description of which is in the top comments in code. Also, this code is written in a much longer way that is probably necessary, but it's easy for me to read, and since I find coding to be the most challenging aspect, I don't mind writing it in a longer way if its easier for me to understand.

Anyway, Im hoping that if there are any glaring errors that are going to pop up, someone will notice them now and tell me what areas (perhaps debouncing?) I need to study to make this work 100% as intended.

//2 signal Wire controlled H-Bridge. Analog-Linear setup
// 0v = full reverse, 2.5v = stop 5v = full forward

#define LeftMotorSig 9  //pwm outputs
#define RightMotorSig 10 //pwm outputs

#define LeftNeutral 8 // neutral indicator left
#define RightNeutral 7 // neutral indicator right

#define LeftReverse 0  // button in
#define LeftForward 1 // button in
#define RightReverse 2 // button in
#define RightForward 3 // button in

int LRVal = 0;
int LFVal = 0;
int RRVal = 0;
int RFVal = 0;

void setup ()
{
  pinMode(LeftMotorSig, OUTPUT);
  pinMode(RightMotorSig, OUTPUT);
  pinMode(LeftNeutral, OUTPUT);
  pinMode(RightNeutral, OUTPUT);
  Serial.begin(9600);
}
void loop ()
{
  LRVal = analogRead(LeftReverse);
  LFVal = analogRead(LeftForward);
  RRVal = analogRead(RightReverse);
  RFVal = analogRead(RightForward);
  
int LRVal2 = LRVal/8;
int LFVal2 = LFVal/8;
int RRVal2 = RRVal/8;
int RFVal2 = RFVal/8;

if ((LRVal2 == 0) && (LFVal2 == 0)) {
  analogWrite(9, 127);
  digitalWrite(LeftNeutral, HIGH);
} else {
  digitalWrite(LeftNeutral, LOW);
}
if ((RRVal2 == 0) && (RFVal2 == 0)) {
  analogWrite(10, 127);
  digitalWrite(RightNeutral, HIGH);
} else {
  digitalWrite(RightNeutral, LOW);
}
if (LFVal2 > 0)
analogWrite(9, LFVal2 + 127);

if (LRVal2 > 0)
analogWrite(9, 127 - LRVal2);

if (RFVal2 > 0)
analogWrite(10, RFVal2 + 127);

if (RRVal2 > 0)
analogWrite(10, 127 - RRVal2);
}

No glaring errors, but you've declared names for the PWM pins, so why not use them?

Also LRVal etc have global scope, but you've given local scope to LRVal2 and the other variables. Is there a reason for this?

No glaring errors, but you've declared names for the PWM pins, so why not use them?

Also LRVal etc have global scope, but you've given local scope to LRVal2 and the other variables. Is there a reason for this?

Im super new to this (approx 3 weeks), and I just started writing and modifying as I figured it out. Also, some of the global scope stuff is very confusing to me because a lot of the literature out there is for older arduino software, so I see variables defined as const int and #define and it gets me very confused, Basically I cobbled together a bunch of the example and tutorial code, so things like that are my shortcomings as a coder, not an intentional thing.

TBH - I dont really understand what you mean by I've declared names for the PWM pins, so why not use them. Do you mean use the names I declared? If so, thats another "me learning as I go thing" If you mean something else, can you please explain?

#define LeftMotorSig 9  //pwm outputs
#define RightMotorSig 10 //pwm outputs

#define LeftNeutral 8 // neutral indicator left
#define RightNeutral 7 // neutral indicator right
void setup ()
{
  pinMode(LeftMotorSig, OUTPUT);
  pinMode(RightMotorSig, OUTPUT);
  pinMode(LeftNeutral, OUTPUT);
  pinMode(RightNeutral, OUTPUT);
  Serial.begin(9600);
}

//good so far
void loop ()
{
  LRVal = analogRead(LeftReverse);
  LFVal = analogRead(LeftForward);
  RRVal = analogRead(RightReverse);
  RFVal = analogRead(RightForward);
// ok , still good (apart from the scope of LRVal


  analogWrite(9, 127);

//Oh dear...  what is '9'?

so I see variables defined as const int and #define and it gets me very confused

Intrinsically, those things have very little to do with scope. "LRVal" is declared outside of "loop" and "setup", so its value survives from loop to loop. Unless you really need the value to survive, you should declare variables with the tightest possible scope. No big deal, but it is a useful skill to acquire.

I see what you mean, thank you for explaining it to me. Coding is very frustrating for me because I am mechanical/hands on by trade. I figure I'll just keep writing sketches and asking questions and I'll pick it up step by step. I spent a couple weeks with a code book and I probably learned more in a day by just trying to write some code and asking questions (so long as you guys will continue to take pity on me and explain, like you did above ;) )

Thanks again.

I probably learned more in a day by just trying to write some code

Book learning programming is hard, but like most learning experiences, doing, and just as importantly, failing (and you will fail along the way) is the way we all learn.
However, our creations are more easily built and torn down than any mechanical device.

Im no stranger to failing, thats for sure! I got this breadboarded out, and it was not working as intended. I was getting 2.5v at the pwm pins with nothing pressed, but with buttons pressed It would always go LOW on PWM when one button should have made it go HIGH. I had a pull down resistor on the LOW button, do I need a pull up resistor on the buttons that are supposed to increase the signal? (I may have used pull down/pull up backwards there)

but with buttons pressed

  LRVal = analogRead(LeftReverse);
  LFVal = analogRead(LeftForward);
  RRVal = analogRead(RightReverse);
  RFVal = analogRead(RightForward);

Buttons? As in on/off switches? Connected to analog pins? I'm confused. Or, you are. Or we both are.

Buttons? As in on/off switches? Connected to analog pins? I'm confused. Or, you are. Or we both are.

Well, I am currently Always confused, so at least that part is true. I know I've got a similar thread going in the hardware section but i made it after this one, because all signs point to hardware problems. Anyway, they are variable resistance switches from a console controller, cut from the main board at the ribbon cable and wires soldered to the ribbon.

all signs point to hardware problems.

The thing to do in a case like this is to test all of your assumptions. What values are you reading from the switches. Perhaps not what you think you are.

int LRVal2 = LRVal/8;
int LFVal2 = LFVal/8;
int RRVal2 = RRVal/8;
int RFVal2 = RFVal/8;

Why divide by 8? OK, looking ahead, I see that the idea is to get a value in the range 0 to 127. But, it may be necessary to adjust this based on the actual (range of) value(s) read from the switches. So, the previous question really needs to be answered/ assumption validated before the rest of the code can be effectively analyzed.

Why divide by 8? OK, looking ahead, I see that the idea is to get a value in the range 0 to 127. But, it may be necessary to adjust this based on the actual (range of) value(s) read from the switches. So, the previous question really needs to be answered/ assumption validated before the rest of the code can be effectively analyzed.

Well, I didn't post it here, tho i did post it in the hardware thread, but I shortened it down to two buttons to just try to troubleshoot. I changed the High side button (was 127 + analogreadval/8) to just strictly analogwrite( 9/leftmotorsig, 255) to make sure I wasnt going over 255 to 256 and looping back to zero. Also, thats why I have serial.begin there, I may have deleted it from the code but I was watching the buttons go from 0-128 (not instantly but with button pressure, IE half pressure would give 64) and it was working.

So I think I've either got a hardware problem or I need to enable the pulldowns on the analog pins. I just picked up a pair of matching pots and Im going to try and rewrite the code using them to see if I can get a better understanding of what's going on.

If there are other things I need to look into before I can declare it a hardware/wiring problem, please inform what else I need to look into. As always, thanks very much!

I need to enable the pulldowns on the analog pins.

Analog pins don't have pull-up resistors, so, you won't be doing this.

If you are getting numbers from the switches, after dividing by 8, that range from 0 to 127, then the switches are working, are wired correctly, and are being read correctly. So, we can concern ourselves with the rest of the code.

The code is intended to control an H Bridge, description of which is in the top comments in code.

The inputs to an H-Bridge are typically speed and direction.

#define LeftMotorSig 9  //pwm outputs
#define RightMotorSig 10 //pwm outputs

#define LeftNeutral 8 // neutral indicator left
#define RightNeutral 7 // neutral indicator right

These names do not accurately describe the purpose of the pins. Therefore, later in the code, it becomes difficult to translate these names to the real meaning of the variable, and, therefore it is difficult to follow the code.

Of course, it's some

if ((LRVal2 == 0) && (LFVal2 == 0)) {
  analogWrite(9, 127);
  digitalWrite(LeftNeutral, HIGH);
} else {
  digitalWrite(LeftNeutral, LOW);
}
if ((RRVal2 == 0) && (RFVal2 == 0)) {
  analogWrite(10, 127);
  digitalWrite(RightNeutral, HIGH);
} else {
  digitalWrite(RightNeutral, LOW);
}

Use the variables you set at the top, not explicit pin numbers, and comment this code. You have the controller in front of you. Presumably, there are labels on the switches that are irrelevant for the purposes you are using them for, but, I'm also presuming that you have a picture somewhere that defines exactly what LFVal1, LRVAL1, etc. mean. Put that picture in the code as comments. Then, we can see where the code says to do something that disagrees with the comments.

Your car has forward and reverse, right? Do you often set the speed before setting the direction? That's what you are doing here.

I think you would be better off separating the direction selection from the speed selection. Make sure the motors turn the right way, based on the switch or switches selected, first. Then, worry about making sure the speed that they turn at is proportional to the pressure.

Thanks for constructive comments, Ill rewrite and change variable names and comment. The reason i used explicit pin numbers instead of variables is because that was how it was done in the various tutorial examples.

As for speed vs direction, I am using 2 motors, one on either side, so to turn left, left side motor goes in reverse, differential/tank drive, so dont they have to be set at the same time?

are you saying I should maybe rewrite using digital input pins and not worry about analog pins until I have directions correct from full high/low?

You need to set speed and direction just like you do in a car. You do not get in, and press the accelerator half way, then shift into reverse. You shift into reverse first, then set the speed by pressing the accelerator. Do the same with the tank. Set the direction, first, then the speed.