Go Down

Topic: Trouble With Wire Library (Read 214 times) previous topic - next topic

tdurand29

Apr 17, 2019, 02:10 am Last Edit: Apr 22, 2019, 05:57 pm by tdurand29
I have been working on this project for work for several months. This is my first project working with Arduinos. I'm Making a machine to dispense glue onto a disk while rotating the disk at the same time to get a consistent bead all the way around. I am using two stepper motors. One to turn a lead screw to dispense the glue and one to turn the disk. I quickly realized that I wasn't going to be able to control both stepper motors at the same time at different speeds. This is when I decided to add an arduino uno, the mega as master and uno as slave to control the stepper motor that spins the disk. Ive also incorporated Pots to have manual adjust rpm and steps for the motors, as well as an lcd to display the values. My problem comes into play when I try and use the Wire Library. I use the serial monitor to try and diagnose the problem and i figured out that the master code hangs up at "Wiring". The serial monitor stops and nothing happens after that. I'm not sure how to fix this problem. Does Wire transmission stop the master code from running?

Thank you for your time!

Thomas

***My code was too long so I attached it as a file as well as my slave code.

PaulS

Quote
I quickly realized that I wasn't going to be able to control both stepper motors at the same time at different speeds.
I can. I don't see why you can't. Using the MultiStepper library makes this pretty easy.

Quote
i figured out that the master code hangs up at "Wiring".
Why is Wiring in quotes? Is that a function that you wrote?

Code: [Select]
      int rpm = map(rpmValue, 0, 1023, 0, 200);
map() is a pretty expensive way to divide by 5.

Code: [Select]
void receiveEvent () {
  while (Wire.available()) {
    char c = Wire.read();
    if (c == 'H') {
      Serial.print("SIGNAL");

You are sending one letter messages. Why do you need a while loop to read all one bytes of the one byte messages?

Do you REALLY think that you can sit in an interrupt handler, where interrupts are disabled, and call functions that need interrupts enabled?

ALL that you should be doing in receiveEvent() is reading the wire data. EVERYTHING you do with the data MUST be done after the ISR ends, when interrupts are again enabled.
The art of getting good answers lies in asking good questions.

GolamMostafa

You are sending one letter messages. Why do you need a while loop to read all one bytes of the one byte messages?

Do you REALLY think that you can sit in an interrupt handler, where interrupts are disabled, and call functions that need interrupts enabled?

ALL that you should be doing in receiveEvent() is reading the wire data. EVERYTHING you do with the data MUST be done after the ISR ends, when interrupts are again enabled.
Code: [Select]
void receiveEvent () {
  while (Wire.available()) {
    char c = Wire.read();
    if (c == 'H') {
      Serial.print("SIGNAL");


When the constraints of the above quote are applied on your codes, the codes turn into:
Code: [Select]
void receiveEvent()
{
   c = Wire.read();     //c should be declared in global area as volatile char c;
   flag1 = true;          //flag1 should be decalred in global area with initial status of false
  
}


Use the value of flag1 in the loop() function to test variable c against 'H' and then take decision as to invoking  the Serial.print() command.

tdurand29

Thank you for your responses!

I got rid of the while() and declared c and a volatile char. Now it doesn't hangup and it continues to go through code but it will only allow me to press the glue button once. after the first wire transmission on the second attempt it hangs. Do I have to reset c? Also I'm particularly sure how to use flags or why I would need them. If you could please explain it would be appreciated. I haven't changed anything in my master as of now. Also sometimes it just hangs up after the wire transmission when its looping the steps for the gluing stepper motor. Could this be caused by the code crashing the mega?

I'll attach the new slave code.

Thank you!

PaulS

Quote
I'll attach the new slave code.
We're waiting...
The art of getting good answers lies in asking good questions.

tdurand29

#5
Apr 18, 2019, 06:17 pm Last Edit: Apr 18, 2019, 06:22 pm by tdurand29
I thought i attached the new code to the initial post but ill put it here as well.

Code: [Select]
#include <Stepper.h>
#include <Wire.h>

volatile char c = 'L';
//setting up pins
int in1Pin = 10;
int in2Pin = 11;
int in3Pin = 12 ;
int in4Pin = 13 ;
const int stepsPerRev = 200;
Stepper turnTable (stepsPerRev, in1Pin, in2Pin, in3Pin, in4Pin);
void setup() {
  Wire.begin(5);
  Wire.onReceive(receiveEvent);
  //setting pin mode
  pinMode(in1Pin, OUTPUT);
  pinMode(in2Pin, OUTPUT);
  pinMode(in3Pin, OUTPUT);
  pinMode(in4Pin, OUTPUT);

  Serial.begin (115200); //starting the serial monitor
}

void loop() {
}
void receiveEvent () {
  Wire.available();
  c = Wire.read();
  if (c == 'H') {
    Serial.print("SIGNAL");
    int rpmValue = analogRead(A3);
    int rpm = map(rpmValue, 0, 1023, 0, 200);
    turnTable.setSpeed(rpm);
    turnTable.step(2000);
  }
  else if (c == 'L') {
    Serial.print ("L");
  }
}

PaulS

Quote
I thought i attached the new code to the initial post but ill put it here as well.
Why did you replace code full of errors that have been pointed out? You make people that commented on your poor code look stupid, when the code no longer has the errors pointed out. Suck it up, and leave the original crappy code in place.

Put the original code back, if you expect any more help.
The art of getting good answers lies in asking good questions.

tdurand29

#7
Apr 22, 2019, 06:00 pm Last Edit: Apr 22, 2019, 06:25 pm by tdurand29
Sorry for replacing the code. I wasn't trying to make anyone look stupid or offend you. I've only done a few posts and still learning the educate. I'm sure all of my code is crap, even the updated one. haha

The Original code is back on the original post.

PaulS

Quote
I'm sure all of my code is crap
Now that I have read it, so am I.

There are things you can do in interrupt service routines. And, there are things that you can't do.

You are trying to do everything in the ISR. Stepping a stepper motor n times is NOT something you do in an ISR.

Code: [Select]
  Wire.available();
Let's ask how much data is available to read, but stick our fingers in our ears so we don't hear the answer. Not a good idea.

The art of getting good answers lies in asking good questions.

tdurand29

I get that you're trying to help me. The reason I'm asking for help in the first place is because I don't understand the wire library. I've read about it and have tried researching it on my own but have not gathered anything to help. Is there a fix? or you just telling me not to use wire? At this point I'm not necessarily looking for the best/right way to code this. Like I said this is my first project and I don't write code for a living. I'm just trying to get this working in a repeatable and reliable fashion.

PaulS

Quote
At this point I'm not necessarily looking for the best/right way to code this
You SHOULD be looking for the right way. The best way could come later.

Do as everyone else does that receives data. Do NOTHING more in the ISR than read the data and store it in a global array. If the presence of data is not sufficient, you can set a flag, indicating that the ISR has stored data. As soon as you see the flag set, in loop(), copy the data and clear the flag. You might want to disable interrupts while you do that, depending on how much data needs copying, and enable them again afterwards.

As for the available() call, there is no reason to call available() if you don't care how much data is available to read. If you do care, and you should, store the returned value in a variable.
The art of getting good answers lies in asking good questions.

tdurand29

If the presence of data is not sufficient, you can set a flag, indicating that the ISR has stored data. As soon as you see the flag set, in loop(), copy the data and clear the flag. You might want to disable interrupts while you do that, depending on how much data needs copying, and enable them again afterwards.

As for the available() call, there is no reason to call available() if you don't care how much data is available to read. If you do care, and you should, store the returned value in a variable.
Thanks for the Help. I believe I fixed the problem after doing more research about flags. It is now working more reliably. Now the only problem I'm facing is random characters on the LCD, which I believe is caused by noise or something like that. I need to do some research on using capacitors as filters, but I believe this problem is for another part of the forum.

below is the current slave code that's working.

Code: [Select]
#include <Stepper.h>
#include <Wire.h>

volatile char c = 'L';
volatile bool flag1 = false;
//setting up pins
int in1Pin = 10;
int in2Pin = 11;
int in3Pin = 12 ;
int in4Pin = 13 ;
const int stepsPerRev = 200;
Stepper turnTable (stepsPerRev, in1Pin, in2Pin, in3Pin, in4Pin);
void setup() {
  Wire.begin(5);
  Wire.onReceive(receiveEvent);
  //setting pin mode
  pinMode(in1Pin, OUTPUT);
  pinMode(in2Pin, OUTPUT);
  pinMode(in3Pin, OUTPUT);
  pinMode(in4Pin, OUTPUT);

  Serial.begin (115200); //starting the serial monitor
}

void loop() {
  if (flag1) {
    int rpmValue = analogRead(A0);
    int rpm = map(rpmValue, 0, 1023, 0, 200);
    turnTable.setSpeed(rpm);
    turnTable.step(2000);
    flag1 = false;
  }
}
void receiveEvent () {
  if (c == 'H') {
    Serial.print("SIGNAL");
    flag1 = true;
  }
  else {
    flag1 = false;
  }
}

Go Up