SpenceKonde / megaTinyCore

Arduino core for the tinyAVR 0/1/2-series - Ones's digit 2,4,5,7 (pincount, 8,14,20,24), tens digit 0, 1, or 2 (featureset), preceded by flash in kb. Library maintainers: porting help available!
Other
542 stars 140 forks source link

Ultrasonic Distance Sensors #1080

Open heywalker opened 2 months ago

heywalker commented 2 months ago

I am experiencing an accuracy problem with all of the MegaTinyCore devices I have tested (ATtiny 402, 1614, 1616, 414, 32xx ). This appears to be a core issue, as the same sketch and hardware works properly with DxCore (DB28, DD14, DD20) and Arduino Uno and Nano. Using a slightly modified example Ping sketch from the Arduino IDE (1.8.13) and a common HC-SRO4 ultrasonic sensor aimed at a flat target about 5 inches distance the ATtiny processors give a distance of about 105 inches while the DxCores correctly measure about 5 inches. All are running at 5 Volts and 20 or 24 MHz with default IDE settings. Any suggestions regarding what I am overlooking, or may this really be an issue with the core? Screenshot_ATtiny402 Screenshot_ATtiny1616 Screenshot_AVR64DD20 Basic_Ping_Print.txt

hmeijdam commented 2 months ago

Are you maybe using a HC-SRO4 library that does not support the modern Attinies and subsequently gets all of its timekeeping wrong?

heywalker commented 2 months ago

Nope, no libraries are included in the sketch. I have also observed the issue with other sensors: Ping, RCWL-1601, and US-100. Thanks for the response.

`// Minimal sketch from Arduino "Ping" example // Modified for separate trig and echo pins // Same pins used for all three examples // Most recent cores as of April 4, 2014 // Arduino 1.8.13 on Linux Mint const int trigPin = PIN_PA2; const int echoPin = PIN_PA3; long duration, inches, cm;

void setup() { Serial.begin(115200); delay(5000); // time to toggle display Serial.println("\n\n\n\t\tStartup AVR64DD14"); //Serial.println("\n\n\n\t\tStartup ATTiny1616"); //Serial.println("\n\n\n\t\tStartup ATtiny402"); pinMode(trigPin, OUTPUT); pinMode(echoPin, INPUT); } void loop() { for (int n = 0; n < 10; n ++) { digitalWrite(trigPin, LOW); delayMicroseconds(2); digitalWrite(trigPin, HIGH); delayMicroseconds(5); digitalWrite(trigPin, LOW); duration = pulseIn(echoPin, HIGH); inches = microsecondsToInches(duration); cm = microsecondsToCentimeters(duration); Serial.print("\t"); Serial.print(inches); Serial.print(" in\t"); Serial.print(cm); Serial.print(" cm\t"); Serial.print(duration); Serial.print(" duration\n"); delay(50); } exit(0); } long microsecondsToInches(long microseconds) { return microseconds / 74 / 2; } long microsecondsToCentimeters(long microseconds) { return microseconds / 29 / 2; }`

hmeijdam commented 2 months ago

I am getting the same as you. tested with a AT404 The odd thing is that pulsein gives different feedback when you compile for different clock speeds. E.g. when I compile at 1MHz, the distance measurement is OK.

You could try this for now. Seems to give correct results in all clockspeeds

const int trigPin = PIN_PA2;
const int echoPin = PIN_PA3;
long duration, inches, cm;

void setup() {
Serial.begin(115200);
delay(1000); // time to toggle display
//Serial.println("\n\n\n\t\tStartup AVR64DD14");
//Serial.println("\n\n\n\t\tStartup ATTiny1616");
Serial.println("\n\n\n\t\tStartup ATtiny404");
pinModeFast(trigPin, OUTPUT);
pinModeFast(echoPin, INPUT);
}
void loop() {
digitalWriteFast(trigPin, LOW);
delayMicroseconds(2);
digitalWriteFast(trigPin, HIGH);
delayMicroseconds(10);
digitalWriteFast(trigPin, LOW);
loop_until_bit_is_set(PORTA_IN, PIN3_bp);
uint32_t snapshot_micros = micros();
loop_until_bit_is_clear(PORTA_IN, PIN3_bp);
duration = micros() - snapshot_micros;
inches = microsecondsToInches(duration);
cm = microsecondsToCentimeters(duration);

Serial.print(inches);
Serial.print(" in   ");
Serial.print(cm);
Serial.print(" cm   ");
Serial.print(duration);
Serial.print(" dur\n");
delay(1000);
}
long microsecondsToInches(long microseconds) {
return microseconds / 74 / 2;
}
long microsecondsToCentimeters(long microseconds) {
return microseconds / 29 / 2;
}
SpenceKonde commented 2 months ago

If pulseIn is broken this bug is double-red critical priority, as pulseIn is one of the most highly analyzed pieces of code in this entire core,and is considered the only 100% reliable way to measure time of an external event. It is used in our calibration routines, everything - but I can't see how this sort of error could happen, as we have tested pulseIn at literally every speed that the internal oscillator of the chip works at since that is how we perform all calibration.

Can you get the intermediate values that you're seeing out of this so we can see where this is going off the rails? If pulseIn is not working, until this is confirmed and the fix is implemented (which will be an emergency release), the core should be considered unfit for any application which requires any timekeeping accuracy of any sort.

pulseIn should be the iron clad leaves nothing to chance way of timing something, and is used in numerous places in supporting code with the assumption that it will work under all cases other than misleading the chip about it's operating frequency.Hence, if this is not the case, the foundations on which all confidence in the core's ability to keep time are based on a false assumption, and hence cannot be trusted

heywalker commented 2 months ago

Besides the pulseIn duration included in the examples , and without a scope, what other intermediate values can I get?

heywalker commented 2 months ago

I also see the same with AT412 and AT1616 (Adafruit Seesaw board).

On 04-05-2024 12:45, Hans wrote:

I am getting the same as you. tested with a AT404 The odd thing is that pulsein gives different feedback when you compile for different clock speeds. E.g. when I compile at 1MHz, the distance measurement is OK.

-- Reply to this email directly, view it on GitHub [1], or unsubscribe [2]. You are receiving this because you authored the thread.Message ID: @.***>

Links:

[1] https://github.com/SpenceKonde/megaTinyCore/issues/1080#issuecomment-2040326005 [2] https://github.com/notifications/unsubscribe-auth/A5CXQ2JDVQL4JH6EY5O2GNTY33PL7AVCNFSM6AAAAABFYOE4VCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBQGMZDMMBQGU

hmeijdam commented 2 months ago

I have created a small demonstrator program that requires no external parts

The RTC is used to toggle a pin, because the RTC has it's own clock. Then micros() and pulsein() are used to measure the same pulse.

void setup() {  
  pinModeFast(PIN_PA1, OUTPUT);            // D2=PA1 is OUTPUT LED   (PORTA.DIR |= PIN1_bm) 
  RTC.PITINTCTRL = RTC_PI_bm;    // RTC Interrupt enable
  RTC.PITCTRLA   = RTC_PITEN_bm | RTC_PERIOD_CYC512_gc ;  //RTC Prescaler on and Cycle-xxxx
  Serial.begin(115200);
  Serial.println();
  Serial.print("Pulsemeasurement with CPU frequency: ");
  Serial.print(F_CPU/1000);
  Serial.println("KHz");
  Serial.println();
}

void loop() {
loop_until_bit_is_clear(PORTA_IN, PIN1_bp);
loop_until_bit_is_set(PORTA_IN, PIN1_bp);
uint32_t snapshot_micros = micros();
loop_until_bit_is_clear(PORTA_IN, PIN1_bp);
uint16_t duration = micros() - snapshot_micros; 
Serial.print(duration);
Serial.print(" us via micros()   ");
duration = pulseIn(PIN_PA1, HIGH);
Serial.print(duration);
Serial.println(" us via pulsein()");
delay(1000); 
  }  

ISR(RTC_PIT_vect){              // RTC Interrupt
  RTC.PITINTFLAGS = RTC_PI_bm;  // clear  Interrupt Flag
  PORTA.OUTTGL = PIN1_bm;       // togle Pin/D2
}

//The Realtimer-Counter (RTC) have a own 32,768KHz Cristal.
// 32.768KHz / CYCprescale = periodtime in mS
//
//  BIN         NAME        TIME
// 0b00001001  CYC4        0,122 mS     
// 0b00011001  CYC8        0,244 mS  
// 0b00011001  CYC16       0,488 mS   
// 0b00100001  CYC32       0,977 mS   
// 0b00101001  CYC64       1,953 mS   
// 0b00110001  CYC128      3,906 mS   
// 0b00111001  CYC256      7,813 mS   
// 0b01000001  CYC512      15,63 mS   
// 0b01001001  CYC1024     31,25 mS   
// 0b01010001  CYC2048     62,50 mS   
// 0b01011001  CYC4096    125,00 mS   
// 0b01100001  CYC8192    250,00 mS   
// 0b01101001  CYC16384   500,00 mS   
// 0b01110001  CYC32768  1000,00 mS 

The results are:


Pulsemeasurement with CPU frequency: 20000KHz

15657 us via micros()   49104 us via pulsein()
15645 us via micros()   48832 us via pulsein()
15641 us via micros()   48816 us via pulsein()
15644 us via micros()   48912 us via pulsein()
15645 us via micros()   48592 us via pulsein()
15635 us via micros()   48720 us via pulsein()
15635 us via micros()   48880 us via pulsein()
15638 us via micros()   48704 us via pulsein()
15637 us via micros()   48880 us via pulsein()
15641 us via micros()   48784 us via pulsein()
15647 us via micros()   48976 us via pulsein()

Pulsemeasurement with CPU frequency: 16000KHz

15720 us via micros()   53249 us via pulsein()
15708 us via micros()   53361 us via pulsein()
15712 us via micros()   53377 us via pulsein()
15712 us via micros()   53249 us via pulsein()
15704 us via micros()   53169 us via pulsein()
15712 us via micros()   53393 us via pulsein()
15708 us via micros()   53137 us via pulsein()
15704 us via micros()   53425 us via pulsein()
15716 us via micros()   53297 us via pulsein()
15712 us via micros()   53377 us via pulsein()

Pulsemeasurement with CPU frequency: 10000KHz

15735 us via micros()   24593 us via pulsein()
15531 us via micros()   24513 us via pulsein()
15532 us via micros()   24401 us via pulsein()
15724 us via micros()   24289 us via pulsein()
15735 us via micros()   24417 us via pulsein()
15729 us via micros()   24401 us via pulsein()
15724 us via micros()   24417 us via pulsein()
15724 us via micros()   24337 us via pulsein()
15724 us via micros()   24369 us via pulsein()
15724 us via micros()   24369 us via pulsein()
15548 us via micros()   24561 us via pulsein()

Pulsemeasurement with CPU frequency: 8000KHz

15744 us via micros()   59554 us via pulsein()
15744 us via micros()   59762 us via pulsein()
15736 us via micros()   59618 us via pulsein()
15728 us via micros()   59618 us via pulsein()
15736 us via micros()   59698 us via pulsein()
15752 us via micros()   59730 us via pulsein()
15744 us via micros()   59794 us via pulsein()
15744 us via micros()   59682 us via pulsein()
15752 us via micros()   59698 us via pulsein()

Pulsemeasurement with CPU frequency: 5000KHz

15597 us via micros()   10755 us via pulsein()
15584 us via micros()   10739 us via pulsein()
15597 us via micros()   10835 us via pulsein()
15584 us via micros()   10787 us via pulsein()
15580 us via micros()   10819 us via pulsein()
15591 us via micros()   10755 us via pulsein()
15584 us via micros()   10851 us via pulsein()
15606 us via micros()   

Pulsemeasurement with CPU frequency: 4000KHz

15716 us via micros()   61492 us via pulsein()
15704 us via micros()   61572 us via pulsein()
15708 us via micros()   61604 us via pulsein()
15712 us via micros()   61588 us via pulsein()
15716 us via micros()   61620 us via pulsein()
15712 us via micros()   61476 us via pulsein()
15724 us via micros()   61604 us via pulsein()
15712 us via micros()   61556 us via pulsein()
15732 us via micros()   61636 us via pulsein()

Pulsemeasurement with CPU frequency: 2000KHz

15720 us via micros()   30744 us via pulsein()
15712 us via micros()   30760 us via pulsein()
15712 us via micros()   30760 us via pulsein()
15720 us via micros()   30760 us via pulsein()
15712 us via micros()   30824 us via pulsein()
15720 us via micros()   30744 us via pulsein()
15752 us via micros()   30728 us via pulsein()
15720 us via micros()   30760 us via pulsein()
15752 us via micros()   30808 us via pulsein()

Pulsemeasurement with CPU frequency: 1000KHz

15712 us via micros()   15024 us via pulsein()
15712 us via micros()   15104 us via pulsein()
15712 us via micros()   15024 us via pulsein()
15704 us via micros()   14992 us via pulsein()
15696 us via micros()   15104 us via pulsein()
15704 us via micros()   15024 us via pulsein()
15696 us via micros()   15088 us via pulsein()

I ran this on a 204, 404, 1614, 416, 806 and 1626 all with comparable results as above. All without bootloader.

I have used the Github version of MegaTinycore with a last source fetch date of 29 March 2024

hmeijdam commented 2 months ago

I have tested again with an older version of MegaTinycore (2.6.8) and in that version Pulsein works fine.

2.6.9 is not installable, (CRC error), so this regression took place in 2.6.9 or 2.6.10

Pulsemeasurement with CPU frequency: 20000KHz
15686 us via micros()   15556 us via pulsein()
15678 us via micros()   15576 us via pulsein()
15675 us via micros()   15570 us via pulsein()
hmeijdam commented 2 months ago

I think this commit caused the regression. When I revert the changes of this commit pulsein works ok again. Though at the cost of breaking millis() as 3 other changes were done that day, but the redefinition of microsecondsToClockCycles smells dangerous, as pulsein() is using it.

heywalker commented 2 months ago

And using pulseIn with real sensor at 1, 10, and 20MHz with 2.6.8. Screenshot_ATtiny1616_2_6_8

MX682X commented 2 months ago

I was able to reproduce the Problem, it's fixable by adding the cast to this line in wiring_pulse.c: unsigned long maxloops = (uint32_t)microsecondsToClockCycles(timeout) / 16;

In order to optimize millis, I had to remove the casts from the defines, as they were interfering with some Preprocessor defines. The problem seems to be routed in the compiler failing to do the Math right in that line. Anyway, now I wonder, how to fix this problem: A: Making some new defines that don't have the casts in the defines for millis - this will add about 40 bytes of Flash (measured on the example provided above) B: only add the cast to pulseIn() and don't loose Flash, however, there might be other code out there that relies on this and might be broken aswell. Actually, never mind, compatibility comes first, so going for A

EDIT: PR is out

hmeijdam commented 2 months ago

Actually, never mind, compatibility comes first, so going for A

Agree with you that's by far the best option, or you won't know whose code you are breaking.