Code debugging advice (signal generator)

If code could just do what the fuck it should. Using a Pi Pico.

This


does not equal this

more like

As far as I can tell, everything is fine until it’s actually time to toggle the fucking output.

Even this didn’t get me shit


Should be a simple 5,000 RPM signal… it’s not. Why? Fuck if I know.
fuck me, right?..

I’ve been looking at your post for a couple of minutes and I’m having a hard time figuring out what the problem is – maybe you could explain a little bit more about what you’re trying to do and what seems to be going wrong?

That said, I did notice that the first image contains

sequence[tooth];
if (sequence == 0)

The first statement doesn’t do anything, and the condition of the if statement will always evaluate to false. Could this be part of the problem?

2 Likes

The array named “sequence” represents the order of short and long teeth on a gen 3 LS crank sensor trigger wheel. “tooth++” is suppose to cycle through the array, for the part you quoted to read a 1 or 0 as the current value from the array. If the current value is 0, the low portion of the pulse is longer; if the current value is 1, the high portion is longer. That was the plan anyway.

The point is that doing just sequence[tooth] on it’s own means “take the value with index tooth from the array sequence and don’t do anything with it”. It does not modify the state of the array. Therefore sequence == 0 will always point at the first element of the array (because of how arrays are stored and used).

What you probably want to do here is:

if (sequence[tooth] == 0)
1 Like

I could swear I tried that and got a compiler error. Maybe that was before I realized it’s “==” and not “=” that I needed to type.

1 Like

There were some 1s representing the output being high.


To make sure we’re on the same page

int TPS = 0;
//int CPS = 0;
int sequence[48] = {0, 1, 1, 1, 1, 1, 0, 1, 1, 1, 0, 0, 1, 1, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 0, 1, 1, 1, 0, 0, 1, 1, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0};
//int sequence[48] = {1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 1, 1, 0, 0, 1, 1, 1, 0, 1, 0, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 1, 1, 0, 0, 1, 1, 1, 0, 1, 0, 1, 1, 1, 1};
//int cam[48] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1};
int tooth = 0;
int low = 0;
int high = 0;
int RPMlow = 0;
int RPMhigh = 0;
int math = 1;
int halfdone = 1;
int done = 1;
int previousmicros = 0;
void setup() {
pinMode(A0, INPUT);
//pinMode(A1, INPUT);
//pinMode(A2, INPUT);
//pinMode(A3, INPUT);
pinMode(0, OUTPUT);
//pinMode(1, OUTPUT);
//pinMode(2, INPUT);
//pinMode(3, INPUT);
Serial.begin(921600);
}
void loop() {
int currentmicros = micros();
if (halfdone == 0) {
  if (currentmicros - previousmicros >= RPMlow) {
      halfdone = 1;
      done = 0;
      digitalWrite(0, 0);
      previousmicros = currentmicros;
    }
  }
if (done == 0) {
  if (currentmicros - previousmicros >= RPMhigh) {
      done = 1;
      math = 1;
      digitalWrite(0, 1);
      previousmicros = currentmicros;
    }
  }
if (math == 1) {
tooth++;
if (tooth == 48) {
  tooth = 0;
  }
TPS = analogRead(A0); 
if (sequence[tooth] == 0) {
  low = 1;
  high = 2; 
  }
else {
    low = 2;
    high = 1;
  }
halfdone = 0;
math = 0;
RPMlow = low * TPS;
RPMhigh = high * TPS; 
Serial.print(" TPS=");
Serial.println(TPS);
Serial.print(" tooth=");
Serial.println(tooth);
} 

//Serial.print("low");
//Serial.print(RPMlow);
//Serial.print("high");
//Serial.print(RPMhigh);
//Serial.print(" TPS=");
//Serial.print(TPS);
//Serial.print("A1");
//Serial.print(analogRead(A1));
//Serial.print("A2");
//Serial.print(analogRead(A2));
//Serial.print("A3");
//Serial.print(analogRead(A3));
//Serial.print(" D0=");
Serial.println(digitalRead(0));
//Serial.print("D1");
//Serial.print(digitalRead(1));
//Serial.print("D2");
//Serial.print(digitalRead(2));
//Serial.print("D3");
//Serial.print(digitalRead(3));

}

I know you’ve asked for debugging advice, but I would suggest that your code is far too complicated for (at least how I’ve understood) what you’ve said you’re trying to do and probably needs a rethink. I don’t understand your application well enough to suggest how I would do it, but I would suggest a fixed-RPM version where the long tooth period is twice the short tooth period might look something like

#define CRANK_SENSOR_HIGH_US xxx
#define CRANK_SENSOR_LOW_SHORT_US xxx

int long_teeth[48] = {0, 1, 1, 1, 1, 1, 0, 1, 1, 1, 0, 0, 1, 1, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 0, 1, 1, 1, 0, 0, 1, 1, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0};
int tooth = 0;

void setup() {
  pinMode(0, OUTPUT);
}

void loop() {
  digitalWrite(0, 1);
  delayMicroseconds(CRANK_SENSOR_HIGH_US);
  digitalWrite(0, 0);
  delayMicroseconds((1 + long_teeth[tooth]) * CRANK_SENSOR_LOW_SHORT_US);

  tooth++;
  tooth %= sizeof(sequence) / sizeof(sequence[0]);
}

(caveats: I have not tested it, I haven’t touched any Arduino stuff in a long time and there’s a specific PWM API that is probably conventional style in this instance)

If you add some extra code reading your TPS value and adjusting the delays appropriately, that sounds like it should do what you’re after?

I’ll settle for something delay based, but I was hoping to send the crank single to the ECU and record the serial monitor output (separte program on PC), which would read injector and ignition pulse for cylinder 1 from ECU. Delays would interfere with recording. I can use a separate arduino though.
At a glance, I want to say that won’t create the signal I want, because the 1st delay looks like a static value not referencing anything else, while the second has some math. That last line looks like greak to me. But I know conceptual logic flow, not syntax.
Might be worth clarifying that, at a given RPM, the low transitions (falling edges) are evenly spaced, the high transition varies, based on that pattern of 1s and 0s. Some of that “sizeof” does look like something worth looking into though.

Keep in mind that the Pico has an extra core and the PIO blocks! Even without that, though, you can send your values and just delay for the calculated amount minus the amount of time it took to send your values.

Yeah, again, I just meant it as an example at static RPM. It’s up to you to fill in the rest :wink:

That last line just makes sure that tooth wraps around and you don’t read long_teeth (which I accidentally left as sequence there) out of bounds. sizeof evaluates at compile-time to the size in bytes of the expression, so sizeof(long_teeth) / sizeof(long_teeth[0]) is the same as writing 48 except you don’t have to update it if you change the size of long_teeth.

2 Likes

For this, I’m hoping to keep it board agnostic as much as possible, at least for the crank signal generator. If I ever get around to making my own ECU code though, yeah, PIO for sure.

Speeduino nonsense all over again. As my experience usually goes. It compiler/firmware nonsense.
image
I inverted the write polarity, moved the read. Getting the same garbage.
Well, before correcting with

it was a different state of wrong.
But now…


Inverted polarity gives same result.
Look at that and tell me I’m wrong.