Change analog read

[leorleor]

Hi Jacob, thanks for writing this non-blocking analog read. It is useful for me, even just as example code, because I am generating PWM signals and doing other time sensitive things on Arduinos.

What do you think of this code change? It adds a line in the blocking analogRead so it can wait for an ongoing read to complete. Was curious if you already considered this, as you mention hanging there as one of the few issues with the API.

Peace, Leor

[chipkitbot]

Can one of the admins verify this patch?

[majenkotech]

Hanging was caused by trying to start a conversion on a non adc pin, then waiting for a conversion to finish that had never been started. We need to be sure that this change doesn't do the same thing waiting for something to finish that has never been started in the first place.

[EmbeddedMan]

Who can test that this code change does not have the hanging behavior @majenkotech identified? @JacobChrist ? @leorleor ?

[JacobChrist]

I can look at it on Thursday.

Jacob

On Mon, May 21, 2018, 5:13 PM Brian Schmalz notifications@github.com wrote:

Who can test that this code change does not have the hanging behavior @majenkotech https://github.com/majenkotech identified? @JacobChrist https://github.com/JacobChrist ? @leorleor https://github.com/leorleor ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/chipKIT32/chipKIT-core/pull/405#issuecomment-390823094, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMGa8I9EwBtVWB73nXDc2xOX5DfBTarks5t01g5gaJpZM4Tv6ia .

[JacobChrist]

The change looks sound but I didn't test it today. I'm not working tomorrow so I will have test it first thing in the morning. I'll also try to whip out a multi channel scan using non-blocking ADC code that we can add to the github.io pages. @leorleor I did not think of this, great idea and makes sense to prevent confusion (lockup) among learners.

[EmbeddedMan]

ok to test

[JacobChrist]

@leorleor have you tested this? After a day of banging my head I got this code to compile at the head and I'm testing it on a Fubarino Mini running at 48MHz. I have test three senerios and one hangs (when it should not)

• non-blocking code works fine
• blocking analogRead() hangs when used alone.
• blocking analogRead() will work if a single analogReadConversionStart() is inserted in setup()

I suspect the issue is that prior to a analogReadConversionStart() being called the ADC may not be turned on and the check for conversion complete has never been occurred. My test code is below will cause hang the board:

// Tested with: chipKIT-core 2.0.3, Arduino IDE 1.6.12
// This is in wiring.c, we shouldn't have to redefine it here (issue #288)
#define CORETIMER_TICKS_PER_MICROSECOND    (F_CPU / 2 / 1000000UL)

uint8_t channel = 0;
uint8_t channels[] = { A0, A1, A0, A2, A0, A3, A0, A4 };
uint16_t results[sizeof(channels)];

// test for lockup on analogReadConversionComplete() if analogReadConversionStart ()
// has not been run

void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);           // start serial for output
  while (!Serial);                // Wait for USB Serial to open
  delay(250);
  Serial.println("chipKIT-core ADC speed test");
//#define FAST_ADC
#ifdef FAST_ADC
  analogReadConversionStart(channels[channel]);
#endif
// Uncomment the following line to prevent the crash
//  analogReadConversionStart(channels[channel]);
}

void loop() {
  // put your main code here, to run repeatedly:
  static uint32_t start_us = readCoreTimer();
  static uint32_t stop_us = readCoreTimer();

  uint32_t value;

#ifdef FAST_ADC
  if ( analogReadConversionComplete() ) {
    results[channel] = analogReadConversion();
    channel = (channel + 1) % sizeof(channels);
    analogReadConversionStart(channels[channel]);
  }
#else
  results[channel] = analogRead(channels[channel]);
  channel = (channel + 1) % sizeof(channels);
#endif

  stop_us = readCoreTimer(); //micros();

  // Display Loop Time
  Serial.print("ADC_TEST_PINs: ");
  for (int i = 0; i < sizeof(channels); i++) {
    Serial.print("results[");
    Serial.print(i, DEC);
    Serial.print("]=");
    Serial.print(results[i], DEC);
    Serial.print(" ");
  }
  Serial.println("");
  Serial.print("F_CPU: ");
  Serial.print(F_CPU, DEC);
  Serial.print(" ");
  float us = (float)(stop_us - start_us) / (float)CORETIMER_TICKS_PER_MICROSECOND;
  Serial.print(us, 3);
  Serial.print(" us, ");
  Serial.print(1.0 / us * 1000000.0, 3);
  Serial.print(" Hz");
  Serial.println("");
  delay(1000);

  start_us = readCoreTimer(); //micros();

}

[JacobChrist]

Here is much simpler code that hangs with this pull request:

// Tested with: chipKIT-core 2.0.3, Arduino IDE 1.6.12
#define ADC_TEST_PIN A0

void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);           // start serial for output
  while (!Serial);                // Wait for USB Serial to open
  delay(250);
  Serial.println("chipKIT-core ADC speed test");
  // Uncomment the following line to prevent the crash
  //analogReadConversionStart(channels[channel]);
}

void loop() {
  uint32_t value;

  value = analogRead(ADC_TEST_PIN);

  // Display Loop Time
  Serial.print("ADC_TEST_PINs: ");
  Serial.print(value, DEC);
  Serial.println("");
  delay(1000);
}

[JacobChrist]

The ADC is turned on in analogReadConversionStart():

    /* Turn on ADC
    */
    AD1CON1SET  =   0x8000;

In analogReadConversionComplete() the ADC1CON1 bit 0 is checked for DONE-ness.

return (AD1CON1 & 0x0001);

The PIC32 Section 17. 10-bit ADC datasheet states the following about this bit:

bit 0 DONE: Analog-to-Digital Conversion Status bit(2) 1 = Analog-to-digital conversion is done 0 = Analog-to-digital conversion is not done or has not started Clearing this bit will not affect any operation in progress.

Because a conversion has not yet started it looks like it reads 0 (which causes the board to hang).

I didn't look at the PIC32MZ code or the 12-bit ADC's, but his maybe the same or a similar situation.

One possible ways to resolve this might be to turn on the ADC when the board is initialized. Thanks to MX, WiFire, MZ EFC and MZ EFG parts this may mean it would have to be turned on four different ways to fully implement. However, even a partial implementation for MX and MZ parts may have benefits.

[JacobChrist]

@leorleor if you have the time, plug away at this and try to solve it. For me, its not worth the battle. If a beginner needs to use analogRead() and they don't know about non-blocking functions then they will never have this issue. If a user runs across the non-blocking functions then hopefully they are smart enough not to hang themselves. I think, however, maybe a better warning on the http://chipkit32.github.io/chipKIT-core/api_analogread_non_blocking page about the possible hanging when mixing non-blocking with blocking is in order. I'll open another issue (#412).

[JacobChrist]

Why is the check failing on this build. The code locks the board but I would think that the checks should be passing (but I don't know exactly what that means)?

[EmbeddedMan]

That refers to chipKITbot and it's ability to build our code. You can ignore it - the code is not the problem.

*Brian

On Thu, Jun 7, 2018 at 3:35 PM, Jacob Christ notifications@github.com wrote:

Why is the check failing on this build. The code locks the board but I would think that the checks should be passing (but I don't know exactly what that means)?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/chipKIT32/chipKIT-core/pull/405#issuecomment-395556029, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbeCDR49K2pAr7TpEQXl04BtLtoyTLHks5t6Y6NgaJpZM4Tv6ia .

[chipkitbot]

Can one of the admins verify this patch?

[EmbeddedMan]

OK to test

On Fri, Aug 24, 2018 at 11:47 AM chipKIT Bot notifications@github.com wrote:

Can one of the admins verify this patch?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/chipKIT32/chipKIT-core/pull/405#issuecomment-415816256, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbeCCCwJ1B7qn0-xdKHhr8IViJAE7dbks5uUC4TgaJpZM4Tv6ia .