I2C speed issues

Hi there!

Let's start off by saying I'm an ultra beginner in programming. I am at the point of copying code and smashing two things together, adding the super basic stuff. The code I am trying to write has to get ESP8266 (D1 mini clone) talking, one taking as much analog reads as possible and spitting out the highest value within a period of time over I2C to the second. I was trying to see how many times the code runs between Wire.writes, and with a request every approximate 10ms the void loop runs about 1500 times, but when I paste in my analogRead code in the loop keeping track of time and the highest value taken it's all over the place. It seems to slow the code down a LOT but also miss a heck of a lot of request events. It now runs between 92 and 1182 from what I've seen in the serial monitor.

What am I doing wrong here?

#include <Wire.h>

int timePassed = 0;

int val;
int highVal;

int mills;
int millsLast;
int highResetInterval = 20;
 
enum REQUEST_TYPE {
    NONE = -1,
    GET_NAME = 0,
    GET_VAL
};
 
void requestEvent();
void receiveEvent(int numBytes);
 
REQUEST_TYPE request = NONE;
 
void setup() {
    Wire.begin(0x12);
    Serial.begin(115200);
    while (!Serial){}
    Wire.onReceive(receiveEvent);
    Wire.onRequest(requestEvent);
}
 
void loop() {
  mills = millis();
  val = analogRead(A0);
  
  if(mills - millsLast >= highResetInterval){
    highVal = val;
    millsLast = mills;
  }

  if(val>highVal){
    highVal = val;
  }

  timePassed++;
}
 

void requestEvent() {
    switch (request) {
        case GET_VAL:
            Wire.write((byte)highVal);
            Serial.println(timePassed);
            timePassed = 0;
            request = NONE;
            break;
        default:
            break;
    }
}


void receiveEvent(int numBytes) {
    if (numBytes==1){
        int requestVal = Wire.read();
        request = static_cast<REQUEST_TYPE>(requestVal);
    }else{
    }
}

We all started there, you are taking a big step. Simple things that may help.
The hardware is dumb and does nothing. The software (firmware) cannot do anything.
Put them together in the correct combination and it will accomplish a lot.

However the processor knows nothing about time, it simply reads an instruction and executes it, then it goes to the next. It steps only as fast as its clock will take it. It cannot process more then one instruction at a time and the one before it has to finish first. It does them very fast and we being slower it appears they are happening concurrently but they are not.

Interrupts grab the processor as soon as it has finished its current instruction and branches to the code for it. All other process stop until finished. Some interrupts can be interrupted by more important ones.

When a processor is reset it sets its internals to a known state and gets its first instruction from specific location in memory (Flash in Arduino). That code sets the hardware to predefined values then check if it is to run user code. If found it jumps to it, if not it does its thing. This allows you to program the part and run it while connected to your computer then later just power it up and it will run your code without a computer, the compiled binary of your code is in memory, a form which the computer understands but it does not resemble what we entered as source code. The compiler converted the code.

Assume you are on a freeway with a lot of traffic and 100 speed limit. You cannot go faster then the cars in front of you which are going 40. That is somewhat like serial communications, You cannot do the next until the first is finished. I2C is like that and is much slower then the processor so you just wate the same as if you were in traffic.

Delay behaves the same way except the traffic has stopped until the time as expired.

The analog to digital converter takes time so the analog reads will take time, the processor just sits until it is ready.

Hopefully this helps.

Thanks a lot for the detailed response! What I however still don't understand, to keep with your freeway example, apparently there is a speed limit of 1500 cycles per 10ms. I then introduce traffic in the form of this code.

  mills = millis();
  val = analogRead(A0);
  
  if(mills - millsLast >= highResetInterval){
    highVal = val;
    millsLast = mills;
  }

  if(val>highVal){
    highVal = val;
  }

This code is slowing everybody down, but in my mind it should be constant. It used to be 1500, now it sometimes is 92, sometimes it's 1200. How is that possible? What is the variable here?

Part 2 of the question would be if there is anything to be optimized to make it faster. It's vital I get as much read data as possible, the peaks I'm trying to capture are very short so the faster the code runs the better.

Thanks!

Any code that uses the millis timer must be of the unsigned long type in order for it to work correctly. Otherwise it doesn't wrap round correctly.

Other things that slow you down is the analogRead function, this takes just under 10mS to do.

The other thing is this bit of code

If the loop function takes less time to execute than the value of the variable highResetInterval then the if statement will never see a true value because the value of the variable millis is set every iteration of the loop function.

Also be aware that the compiler can optimise out totally code it thinks is doing nothing. So simple loops that do nothing with a result end up not being in the final machine code at all. Which might explain why when you add stuff:-

There may be other stuff I have missed as well.

Unsigned long is fair enough indeed, changed that.

I don't see how the if statement will never see a true value. The mills value will be updated so if the delta between the last time (in ms) highVal has been reset is less than the highResetInterval it will not do anything that iteration but at some point the delta between mills and millsLast will be equal to or larger than the highResetInterval value. It's basically the the BlinkWithoutDelay example sketch.

I also can't find the analogRead taking more than 10ms. I wrote this code to check how long it takes

unsigned long mils;
unsigned long milsLast;

unsigned long totalDelta;
unsigned long measurements;

int average;


void setup() {
  Serial.begin(115200);
}

void loop() {
  int value = analogRead(A0);
  mils = millis();
  totalDelta = totalDelta + (mils - milsLast);
  milsLast = mils;
  measurements++;
  average = totalDelta/measurements;

  Serial.println(average);
  //delay(1);
}

and the average tracks perfectly with the delay, so if I put a 15ms delay in the average is 15ms, if I set it at 8 it's 8, at 1 it's 1 but without the delay and with the analogRead it sits at a steady 0. What confuses me even more I guess is why the number I get in the other code isn't stable. It stutters, even in it's serial prints, and is all over the place. There must be a way to improve the code to make it faster and stop the stuttering inconsistency right?

What do you expect at this point. Do you really ignore multiple bytes received?

And keep in mind that Serial.print() will block the controller if the output buffer is full.

That is because I said:-

Note under not over.

Also note in the last code you have a Serial.print inside the loop, that takes a lot of time as well.

analogRead() takes less than 100 microseconds. Is there something in the sketch that would cause it to be slower in this case?

a7

That was my understanding too. 100microseconds checks out perfectly with other tests, so I'm unsure what is slowing everything down that much. It's noticable the code is stumbling, the print timePassed (which is a dumb name, it should be cyclesPassed or something) sometimes takes at least 50ms but probably even 100ms or more. It's easily noticable, while it should output every 10ms (since there is a request every 10ms). What makes everything so clunky?

And sorry, that's fair enough, I meant to say just under 10ms. I can't find it taking over 1ms even, and 1ms already is way too long, a sample frequency of 1khz is guaranteed to miss the peaks I need. I am using two separate boards solely because of this, one has to gather data as fast and as much as possible while the other can be slow and process said data. Is there a different way to speed it up significantly?

You are printing in the callback, which I believe means you are printing inside an interrupt service function, or a function it calls.

That is advised against.

You are using variables between your callbacks and your mainline code, these should be qualified as volatile.

Furthermore, your mainline use of any multibyte variables should be done with interrupts off, most ppl

  noInterruots();
  int myCopy = someVolatileVariabke;
  interrupts();

grab their own copy of such variables and use the copies subsequently. Same for setting the volatile variables.

I have no idea if this will make any difference, but is the kind of thing that can work until it doesn't, so best follow the rules.

I'd drop all printing of any kind and move to some alternative for the feedback, at least for now. Here's where the oscilloscope is useful; clever use of simple digital outputs can also go a long way.

a7

I'm really sorry but I am completely lost. What is a callback? What's the mainline code? I am going to cut the feedback out as soon as I know it works like I want it to work but right now it does not seem to work. What I think I should do is put the code in the void loop between the noInterrupts(); and interupts();?

Me too, mostly, but not around this. :expressionless:

You have two functions:

void requestEvent();
void receiveEvent(int numBytes);

which you wrote code for, but never explicitly call. This is because you hand that ability to something without your code:

    Wire.onReceive(receiveEvent);
    Wire.onRequest(requestEvent);

When that entity needs to, it "calls back" to the functions you have written; at that time they do whatever it is that you want with the information the outside entity provides.

I think that outside entity does so in the context of an interrupt, which is how it was made aware of anything going on with the hardware it is supervising.

Thus, those callback functions of yours runs in an interrupt context; variables used in an interrupt and the code you yourself write must be declared volatile, and access to them done with interrupts off.

I've called your code the mainline code, you know like your loop() and other functions you wrote and actually call explicilty and so forth.

No! That is totally overkilling it, and literally killing any chance the unseen code will function smoothly if at all.

You only have to briefly turn off interrupts to work with the volatile variables.

I cannot but repeat the pattern you missed:

  noInterrupts();
  int myCopy = someVolatileVariabke;
  interrupts();

Interrupts are off for a matter of a handful of microseconds as all that is done during the period where they are off is copying a variable. The rest of the loop runs with interrupts on, which it has always been doing even if you did not realize, or care.

In that way the code you aren't seeing will get the interrupts, possibly delayed by a handful of microseconds, which will make no difference, because if there was an interrupt when you were holding them off, it would be taken immediately you turn interrupts off. They wait.

HTH and I do hope it has anything to do with you real issue(s).

a7