Go Down

Topic: Dont know what i did wrong. (Read 1 time) previous topic - next topic

geelongnewbie

Hi there people im doing my first program for a transmission controller. I have written what i have thought was a good sketch but it does not seem to be functioning when i test it. i have no errors coming up when i verify and compile.

I can read the analog voltage through the serial but thats it. none of the outputs seem to be functioning.

I dont know if i have missed a step in the script i would of thought the verify might of picked it up

i have pasted below the sketch, prehaps someone can see what i did wrong or have some pointers on other ways of sketching it.


Thanks in advance.

Gavin.

// Pin Assignment.
int sensorPin0 = (A0); // Trans Range Switch Output.
int sensorPin1 = (A1); // Throttle Position Sensor.
int sensorPin2 = (A2); // Lock Up Converter Command.
int digitalPin10 = (10); // Solinoid 1 Mosfet Driver.
int digitalPin11 = (11); // Solinoid 2 Mosfet Driver.
int digitalPin12 = (12); // Converter Lock Up Solinoid 7 Mosfet Driver.
int digitalPin3 = (03); // Solinoid 6 Mosfet Driver.
int ledPin = (13); // Solinoid 6 Trigger warning light.
void setup () {
 
  Serial.begin(9600);
 
  // Pin Modes for outputs.
  pinMode(digitalPin10, OUTPUT);
  pinMode(digitalPin11, OUTPUT);
  pinMode(digitalPin12, OUTPUT);
  pinMode(digitalPin3, OUTPUT);
  pinMode(ledPin, OUTPUT);
}
void loop () {
int sensorValue = analogRead(A0);
float voltage = sensorValue * (5.0 / 1023.0);
Serial.println(voltage);
// Solinoid Control.

  if (sensorPin0 = A0 > 205);
  (digitalPin10, HIGH);  (digitalPin11, LOW); // Range Switch Output @ 1.0v - S1 Active (First Gear)
  if (sensorPin0 = A0 > 307.5);
  (digitalPin10, HIGH); (digitalPin11, HIGH); // Range Switch Output @ 1.5v - S1&2 Active (Second Gear)
  if (sensorPin0 = A0 > 410);
  (digitalPin10, LOW); (digitalPin11, LOW); // Range Switch Output @ 2.0v - S1&2 Disabled (Third Gear)
  if (sensorPin0 = A0 > 512.5);
(digitalPin10, LOW); (digitalPin11, HIGH); // Range Switch Output @ 2.5v - S2 Active (Forth Gear)

  // Throttle Position Sensor.
  if (sensorPin1 = A1 < 615);
  (digitalPin3, HIGH);
  (ledPin, HIGH); // When the TPS is below 3.0v S6 active
  if (sensorPin1 = A1 > 615);
  (digitalPin3, LOW);
  (ledPin, LOW); // When the TPS is above 3.0v S6 active

  // Converter Lock Up Command.
  if (sensorPin2 = A2 > 1000);
  (digitalPin12, HIGH);} // When the Lock Up switch @ 5.0v - S7 is Active. (TCC Lockup)

AWOL

#1
Jun 13, 2012, 12:29 pm Last Edit: Jun 13, 2012, 12:31 pm by AWOL Reason: 1
Code: [Select]
 if (sensorPin0 = A0 > 205);

Lose the semicolons.
Fix the comparisons.
Read the sensors (analogRead)


Code: [Select]
    (digitalPin10, HIGH);  (digitalPin11, LOW); // Range Switch Output @ 1.0v - S1 Active (First Gear)

Looks like you're missing some calls to "digitalWrite", and possibly some { } braces too.

Quote
i have no errors coming up when i verify and compile

I'm sometimes surprised what will actually compile too.
"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

majenko

#2
Jun 13, 2012, 12:30 pm Last Edit: Jun 13, 2012, 12:34 pm by majenko Reason: 1
Please format your code inside [ code ] blocks.

Quote

float voltage = sensorValue * (5.0 / 1023.0);


That formula is wrong.  You should be dividing by 1024.0 not 1023.0 as there are 1024 steps in the ADC, not 1023.

Quote

  if (sensorPin0 = A0 > 205);


All your if statements are being terminated with a ;.  This is wrong.  What this is saying is "If such-and-such, do nothing.".  Remove the semi-colons or you will never have it do anything useful.

Also, the comparison in there is complete gibberish.  You are saying that sensorPin0 is assigned the truth value of if A0 is greater than 205.  A0 is just a number (a macro which gets replaced with the number 14).  So you have "sensorPin0 is assigned the value 'is 14 greater than 205?'", which is not a comparison.  I'm not sure quite what you're trying to achieve here.

AWOL

#3
Jun 13, 2012, 12:34 pm Last Edit: Jun 13, 2012, 12:36 pm by AWOL Reason: 1
Code: [Select]
if (sensorPin0 = A0 > 205);
Translation:

Check to see if A0 is greater than 205 (it isn't), so assign the result of the comparison (false / zero) to the variable "sensorPin0".
Test to see if the result of the assignment is true or false (it is false), and do nothing.
"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

AWOL

Quote
That formula is wrong.  You should be dividing by 1024.0 not 1023.0 as there are 1024 steps in the ADC, not 1023.

I'm not so sure - the ADC can't tell the difference between 5V and 5V - (5 / 1024), so I think that yes, it should be 1023.

Flame on. 
"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

geelongnewbie

You guys are great thank you for your inputs, Im just learning the language and seem to be understanding it but just found it strange.





GoForSmoke

BTW, you can get code tags by clicking the # button over the post entry window.
If you highlight your code and click that, it will place the tags around the code same as how you might Bold text with the B button.

This block besides being wrong, can you see the ambiguity?
Code: [Select]

  // Throttle Position Sensor.
  if (sensorPin1 = A1 < 615);  // I can assume you want some code below to run on condition true
  (digitalPin3, HIGH);           // Perhaps only this line?
  (ledPin, HIGH);                  // or maybe this line as well?


Here is correct if both should only run on true:
Code: [Select]

 // Throttle Position Sensor.
 if (sensorPin1 = A1 < 615)   // And still what is inside () is very wrong
 {
   (digitalPin3, HIGH);           // Perhaps only this line?
   (ledPin, HIGH);                  // or maybe this line as well?
 }


The braces { } group all lines between. Use them for conditionals, loops, and case statements.
Also good is to learn arrays and pointers.

Here is a helpful page for beginners, one of many:
http://arduino.cc/en/Reference/HomePage
Nick Gammon on multitasking Arduinos:
1) http://gammon.com.au/blink
2) http://gammon.com.au/serial
3) http://gammon.com.au/interrupts

GoForSmoke


Quote

float voltage = sensorValue * (5.0 / 1023.0);


That formula is wrong.  You should be dividing by 1024.0 not 1023.0 as there are 1024 steps in the ADC, not 1023.


Actually no. I got caught on that one till I think is was Robtillart pointed out I made a trap.

Is 5 * reading / 1024 going to get you 5 with reading of 1023?
Sure, you can add 1;
5 * (reading+1) / 1024
but then what will a reading of 0 get?

5 * reading / 1023 works
Nick Gammon on multitasking Arduinos:
1) http://gammon.com.au/blink
2) http://gammon.com.au/serial
3) http://gammon.com.au/interrupts

majenko



Quote

float voltage = sensorValue * (5.0 / 1023.0);


That formula is wrong.  You should be dividing by 1024.0 not 1023.0 as there are 1024 steps in the ADC, not 1023.


Actually no. I got caught on that one till I think is was Robtillart pointed out I made a trap.

Is 5 * reading / 1024 going to get you 5 with reading of 1023?
Sure, you can add 1;
5 * (reading+1) / 1024
but then what will a reading of 0 get?

5 * reading / 1023 works


Hang on... I'm confused now...

My trusty spreadsheet tells me you're right and I'm wrong (unheard of, I know), yet logic tells me the complete opposite.  Logically, there are 1024 steps in the ADC, so the volt-per-step is Aref/1024, then multiply that volt-per-step by the reading value.  But yes, that gives 4.9951171875 for a reading of 1023...

Damn... Can it be that I have been doing it wrong all these years?

Well, you learn something new every day.  Time to re-write all my software...

GoForSmoke

It got me too. All I could reply to Rob is that he is correct.

Quote
Well, you learn something new every day.  Time to re-write all my software...


LOL, and we've both written constants all through our past code.
Nick Gammon on multitasking Arduinos:
1) http://gammon.com.au/blink
2) http://gammon.com.au/serial
3) http://gammon.com.au/interrupts

majenko


It got me too. All I could reply to Rob is that he is correct.

Quote
Well, you learn something new every day.  Time to re-write all my software...


LOL, and we've both written constants all through our past code.



Anyway, it's all pointless when you just assume that your analog reference voltage is 5V.  Your results are going to be wrong whether you use 1023 or 1024 :P

majenko

Got it!

It just struck me about this 1024/1023 thing.

Quantization!

Divide by 1024 and you end up with the voltage at the lower end of the step.  Divide by 1023 and you get the voltage at the upper end of the step.

You should really divide by 1023.5 so as to get the voltage in the middle of the step :P

GoForSmoke

Without going into integers vs float;
Vcc * reading / 1023.5, what you get when maximum reading is 1023.0?

To be able to get both ends and the middle, from 0 to full, it must be 1023.
I think that the non-intuitive part is because of the zero-base to 1023 as full.

With the ADC in quick read the divider should be 255 instead.  ]:D



Nick Gammon on multitasking Arduinos:
1) http://gammon.com.au/blink
2) http://gammon.com.au/serial
3) http://gammon.com.au/interrupts

Nick Gammon

To make it simpler, let's try to convert the range 0 to 9 (10 values) into 0 to 1.

Code: [Select]

Original    Divide by 9         Divide by 10

0           0                     0
1           0.111111111111111     0.1
2           0.222222222222222     0.2
3           0.333333333333333     0.3
4           0.444444444444444     0.4
5           0.555555555555556     0.5
6           0.666666666666667     0.6
7           0.777777777777778     0.7
8           0.888888888888889     0.8
9           1                     0.9


Assuming that 0 means "off" and 9 means "fully on" (like the ADC would) then you clearly have to divide by 9, not 10, otherwise you can never multiply by your "target" (eg. 5V) and get the correct number back. Otherwise the maximum reading (9 in this case) would indicate we have 4.5V on the ADC, which is clearly wrong.
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Go Up