joan2937 / pigpio

pigpio is a C library for the Raspberry which allows control of the General Purpose Input Outputs (GPIO).
The Unlicense
1.45k stars 410 forks source link

waveforms and SYNC: unexpected behavior? #457

Open dr-matt opened 3 years ago

dr-matt commented 3 years ago

(as mentioned in this raspberry pi forum post)

I'm using the pigpio C interface to generate a pulse signal (basically a single period of a square wave) at varying intervals. The behavior I'd like to see is that if a new pulse is created while the previous one is still being sent, that previous one will not be interrupted, but instead will run to completion before the new one begins.

According to the pigpio documentation here, I think I should use PI_WAVE_MODE_ONE_SHOT_SYNC when calling gpioWaveTxSend() for each pulse:

The SYNC variants wait for the current waveform to reach the end of a cycle or finish before starting the new waveform.

However, if the time between two consecutive pulses is shorter than the period, I have observed that the second pulse can be dropped. And if the time between the two pulses is longer, then both are generated correctly, but after that, new pulses are spontaneously and repeatedly generated at the maximum possible rate.

As an example, suppose my pulse is 50ms high followed by 50ms low. I call gpioWaveTxSend(wave_id, PI_WAVE_MODE_ONE_SHOT_SYNC) at t=0, and then call it a second time at t=200ms. I see a pulse starting at t=0 and a pulse at t=200, as expected, but I then also see pulses beginning at 300, 400, 500, ... repeating endlessly even though I have only called the send method twice.

Here is a small example that produces this behavior for me:

#include <pigpio.h>

int main() {
  gpioInitialise();

  int pin_num = 24;
  int pulse_width_ms = 50;
  int wave_mode = PI_WAVE_MODE_ONE_SHOT_SYNC;

  gpioSetMode(pin_num, PI_OUTPUT);

  // set up a square-wave-like pulse
  gpioPulse_t pulses[2];
  pulses[0].gpioOn = 1<<pin_num;
  pulses[0].gpioOff = 0;
  pulses[0].usDelay = pulse_width_ms*1e3;
  pulses[1].gpioOn = 0;
  pulses[1].gpioOff = 1<<pin_num;
  pulses[1].usDelay = pulse_width_ms*1e3;
  gpioWaveAddGeneric(2, pulses);
  int wave_id = gpioWaveCreate();

  // send first pulse; looks fine
  gpioWaveTxSend(wave_id, wave_mode);

  // send second pulse before first has ended; second pulse does not appear
  gpioSleep(PI_TIME_RELATIVE, 0, 1.5*pulse_width_ms*1e3);
  gpioWaveTxSend(wave_id, wave_mode);

  // wait a bit
  gpioSleep(PI_TIME_RELATIVE, 1, 0);

  // send third pulse; looks fine
  gpioWaveTxSend(wave_id, wave_mode);

  // new pulses spontaneously appear every 2*pulse_width until termination
  gpioSleep(PI_TIME_RELATIVE, 2, 0);

  gpioTerminate();
}

I'm using a raspberry pi 4b with stock raspberry pi OS and v79 of pigpio.

guymcswain commented 3 years ago

After a quick read I don't see anything wrong with you are doing. The use cases that I'm most familiar with will apply a new/different wave ID to sync to. Can you try another wave ID, ie, create two identical waves.

~Another suggestion is to run the pigpio self test, x_pigpio to make sure there is no timing conflict with the library.~ Not necessary as I see that Joan has confirmed your test code result.

dr-matt commented 3 years ago

Thanks! I followed your suggestion to create two waves and then alternate between them. This behaves better in the case of widely separated pulses, in that there is no repeating train of spontaneous waves at the end. However, if a second pulse is requested before the first one is complete, the second is still dropped and never appears.

guymcswain commented 3 years ago

The canonical use case: On each iteration, create a new wave, send it synchronously, wait for the previous wave to complete, delete and repeat.

After your delay(s) you should make sure the wave has completed before requesting another synchronous wave

dr-matt commented 3 years ago

Thank you, knowing the common use case is helpful and makes sense - I'm guessing I should use gpioWaveTxBusy to check the status.

However, I'm then confused about the purpose of the _SYNC variants. Aren't they designed to already do what you described, namely wait until the current wave has completed before sending the new wave, as mentioned in the documentation?

guymcswain commented 3 years ago

The method is called gpioWaveTxAt().

Aren't they designed to already do what you described, namely wait until the current wave has completed before sending the new wave, as mentioned in the documentation?

Yes. But what (I believe) needs to be avoided is to 'sync' to a wave that is already 'synced'. The internal structures are similar to liked lists. I think it's possible to create a 'repeat forever' situation if you are not careful.

With all of that behind us, please send me your updated code snippet that shows unexpected behavior and I will look into it this week.

dr-matt commented 3 years ago

I finally had time to test this again. I think what you're saying is that creating a new wave with a new ID each time a pulse is sent should work properly, because we saw that reusing wave IDs (or using a single ID for all pulses) can cause an infinite loop as you pointed out.

Unfortunately I still see that some pulses are dropped. Here's a small modification that demonstrates this on my system.

#include <pigpio.h>

int wave_mode = PI_WAVE_MODE_ONE_SHOT_SYNC;

void send_wave(gpioPulse_t* i_pulses, const int i_num,
               const double i_sleep_ms) {
  // wait before sending wave
  if (i_sleep_ms > 0) {
    int seconds = i_sleep_ms / 1e3;
    int microseconds = 1e6 * (i_sleep_ms/1e3 - seconds);
    gpioSleep(PI_TIME_RELATIVE, seconds, microseconds);
  }

  // create and send wave
  gpioWaveAddGeneric(i_num, i_pulses);
  int wave_id = gpioWaveCreate();
  gpioWaveTxSend(wave_id, wave_mode);
}

int main() {
  gpioInitialise();
  gpioWaveClear();

  const int pin_num = 24;
  const int pulse_width_ms = 50;

  gpioSetMode(pin_num, PI_OUTPUT);

  // set up a square-wave-like pulse
  gpioPulse_t pulses[2];
  pulses[0].gpioOn = 1<<pin_num;
  pulses[0].gpioOff = 0;
  pulses[0].usDelay = pulse_width_ms*1e3;
  pulses[1].gpioOn = 0;
  pulses[1].gpioOff = 1<<pin_num;
  pulses[1].usDelay = pulse_width_ms*1e3;

  // send first pulse; looks fine
  send_wave(pulses, 2, 0);

  // send second pulse before first has ended; second pulse does not appear
  send_wave(pulses, 2, 1.5*pulse_width_ms);

  // send third pulse after waiting a while; looks fine
  send_wave(pulses, 2, 1000);

  // send non-overlapping pulse pairs; these look good
  for (int i = 0; i < 5; ++i) {
    send_wave(pulses, 2, 4*pulse_width_ms);
    send_wave(pulses, 2, 6*pulse_width_ms);
  }

  gpioSleep(PI_TIME_RELATIVE, 1, 0);

  // send overlapping pulse pairs; only one appears in each pair
  for (int i = 0; i < 5; ++i) {
    send_wave(pulses, 2, 10*pulse_width_ms);
    send_wave(pulses, 2, 1.5*pulse_width_ms);
  }

  gpioSleep(PI_TIME_RELATIVE, 1, 0);
  gpioWaveClear();
  gpioTerminate();
}

My interpretation of the docs was that waveforms could / should be reused, and that using SYNC allows one to send a new wave before the previous one completes. Perhaps I'm still using the library incorrectly?

guymcswain commented 3 years ago

I've run your above example code but truncated it after sending the third wave just to focus on the first problem encountered. ~I found that if I regenerate the pulses object each time before calling your send_wave() that it works. If I'm correct, then it means the gpioWaveAddGeneric() is mutating the pulse_t * argument. Can you please validate this?~ If the second wave is 'synced' immediately then it works. Maybe the sync will not take when applied during the transmission of the last pulse in a wave?

guymcswain commented 3 years ago

Indeed changing

send_wave(pulses, 2, 1.5*pulse_width_ms);

to

send_wave(pulses, 2, 0.5*pulse_width_ms);

appears to make your example work.