search

RIOT-OS / RIOT

RIOT - The friendly OS for IoT
https://riot-os.org
GNU Lesser General Public License v2.1
4.92k stars 1.99k forks source link

drivers/srf04: incorrect values on ATmega based platforms #13079

Closed wosym closed 3 years ago

wosym commented 4 years ago

Description

SRF04 driver (drivers/srf04) does not seem to work properly. In tests/driver_srf04 the values seem incorrect and not correlated to the actual distance. It also feels like they over- and underflow. Also, often it returns "Error: no valid data available". (My guess is that this error should be returned when there is no object to reflect the waves onto, so the distance times out. However, this error also appears when it should definitely by in range) Sample output: 

2020-01-10 14:44:03,540 # D: 54 mm
2020-01-10 14:44:03,598 # D: 60 mm
<...>
2020-01-10 14:44:04,702 # D: 87 mm
2020-01-10 14:44:04,760 # D: 87 mm
2020-01-10 14:44:04,818 # D: 97 mm
2020-01-10 14:44:04,876 # D: 95 mm
2020-01-10 14:44:04,935 # D: 100 mm
2020-01-10 14:44:04,993 # D: 98 mm
2020-01-10 14:44:05,052 # D: 104 mm
2020-01-10 14:44:05,112 # D: 102 mm
2020-01-10 14:44:05,171 # D: 109 mm
2020-01-10 14:44:05,230 # D: 106 mm
2020-01-10 14:44:05,287 # D: 0 mm    <--- Overflow?
2020-01-10 14:44:05,344 # D: 1 mm
2020-01-10 14:44:05,401 # D: 2 mm
<...>
2020-01-10 14:44:07,949 # D: 0 mm
2020-01-10 14:44:08,006 # D: 0 mm
2020-01-10 14:44:08,065 # D: 108 mm
2020-01-10 14:44:08,117 # -2
2020-01-10 14:44:08,150 # Error: no valid data available
2020-01-10 14:44:08,201 # -2
2020-01-10 14:44:08,234 # Error: no valid data available
2020-01-10 14:44:08,289 # -7360
2020-01-10 14:44:08,321 # Error: no valid data available
2020-01-10 14:44:08,376 # -8232
2020-01-10 14:44:08,409 # Error: no valid data available
2020-01-10 14:44:08,468 # D: 103 mm
2020-01-10 14:44:08,526 # D: 49 mm
2020-01-10 14:44:08,584 # D: 55 mm
2020-01-10 14:44:08,642 # D: 44 mm
2020-01-10 14:44:08,700 # D: 44 mm
2020-01-10 14:44:08,752 # -2
2020-01-10 14:44:08,785 # Error: no valid data available
2020-01-10 14:44:08,836 # -2
2020-01-10 14:44:08,869 # Error: no valid data available

This has been tested on an atmega1284p-dev board. We also tested the same sensor on the same board with the same connections, but with a bare metal driver, and this seemed to work properly.

I also tried to write my own test, using the documentation of the SRF04 driver, and here the output value seems to be only increasing. It feels like the callback is never reset, and it uses the same start-value over and over again, but I checked this (briefly), and as far as I can tell this is all done like it's supposed to.

Steps to reproduce the issue

Expected results

Actual results

Update 13/07

Currently it appears to be a compatibility issue with AVR (or 8-bit controllers in general), since the only two boards (that currently have been tested) were Atmegas (328p and 1284p).

gschorcht commented 4 years ago

Ups, I don't have the hardware. I guess you meant @aabadie, he has the hardware according to the Maintainer hardware access list

wosym commented 4 years ago

Let me start off by saying that I no longer have the hardware. This Issue was opened in Januari, and I currently work at a different company.

Second, this sensor is not fault-proof. It is low budget, it is dependent on noise, close reflecting surfaces, etc. and thus measurements can easily be disturbed by its environment.

As mentioned in the issue, the exact same breakout board was tested on the same controller with both the RIOT driver, and a bare metal driver. The BM driver worked as expected, the Riot driver did not produce any valid data. If low accuracy was the issue, both implementations should have been affected.

You can improve results by accumulating multiple measurements (eg. ignoring outliers and averaging the rest).

Fluctuating values were not the issue. Entirely wrong values (and overflows) were the issue.

If your are reading out the samples to fast it will raise Error: no valid data available. This does not necessarily mean there is something broken, in most cases this simply means, the sampling is not done yet.

We used the defaults in the test app

However, this might also mean you are exceeding the sensors maximum range of 3m, which means there will never be an interrupt by the chip and it'll reside in measurement state. The driver can by no means differentiate these two errors.

The described behavior occurred far below the maximum range. (< 50 cm) If I remember correctly, we also tried exceeding the maximum range. It still produced values while it should not. (it overflowed and kept sending out values)

If you suspect this happening you might want to try increasing the 50us, eg. to 55us in following line. If this works for you please let me know

I'd love to test it, but unfortunately I don't have the hardware anymore. I'll try to get a hold of one, but can't make any promises. However... if I remember correctly we did also play with this variable. It did not have any positive effects on the results.

the object to detect should not be to small or have a form that reflects the waves oddly. If this applies to your testing, you might use your hand or a smartphone instead to test if the results become more reasonable.

We used a cardboard box within arms-length away from the sensor. For the long range tests (which exceeded the maximum range) we used a wall.

I think this issue should focus on this problem, so please provide a good description on how to reproduce this measurement: 2020-01-10 14:44:05,287 # D: 0 mm <--- Overflow?

wosym commented 4 years ago

Looks like we're in luck: I checked some scrap boxes, and found three of these modules! Looks like I had some of my own after all... :)

I quickly tested it on a nucleo-f207zg I had laying on my desk. I could not reproduce the behaviour that this issue describes. The readings are very innacurate, yes. But that's to be expected with a $1 sensor board. I did not detect any overflows, no readings that were obviously incorrect and no errors. It works like it should...

So now I start to wonder... might it be something AVR specific? Because that's the only difference between the test I did now, and the test I did back in Januari.

wosym commented 4 years ago

I tested again with an Atmega328p.

It does seem like something is wrong: on every reset it only does 3 or 4 readings, and then stops. enabling DEBUG in the driver does not give any extra info.

However, the data it did print seemed more or less correct. So once again, I could not reproduce the issue described above.

It does reinforce my hypotheses that there is some kind of conflict between this driver and AVR though. Nucleo worked fine, but two different AVR controllers both showed a different kind of bug.

wosym commented 4 years ago

This is odd indeed. I don't really get though why you keep suspecting the driver to be faulty, if your measurements are correct now.

I never made a statement on what the cause of the errors was. I just stated the incorrect behavior. Wether that's because of an error in the driver, in the board config, or something else, is still undefined.

To summarise what I said: Riot-driver on nucleo: works Bare metal driver on AVR: works Riot-driver on AVR: does not work

So clearly, there's still something wrong somewhere in between the driver and the AVR boards in RIOT. Imho the tests should run just fine on all boards, unless there's a good reason why they should not (e.g. ethernet tests on boards with no ethernet capabilities, or test programs that cannot be run on small targets due to memory constraints). In that case it should be documented that the test does not work on this board.

If the value is "wrong", either overflowing the signed integer used for storing (I guess you mean that? You didn't clear this up yet.),

I didn't specify that it was the integer that overflowed. I merely described behaviour that looked like overflows, because it went from a high value to a low value. Even though the measured distance was definitely within range.

I understand that it if you go out of range, it might happen that an old echo is received after the new trigger is made. But... that should not happen if the distance is only 20 cm away. And even if it did, that does not explain why it did not show this behavior on STM and on a bare metal driver.

I could not reproduce this with my atmel samr21-xpro

Neither could I on nucleo, so it's not that strange that you also were not able to reproduce the behavior on a different board. It did however occur on two different AVR boards (m328p and m1284p).

Anyway, I am going to close this issue soonish if there is nothing else.

I personally do not agree that there is no issue here, but I would agree that with the newly gained knowledge, the title of this issue is a little misleading. It implies that there is something wrong with the driver, but in reality there is only an issue when using it on AVR.

wosym commented 4 years ago

The tests have been run, the report of the undesired behavior has been made. There's no point in opening a duplicate issue. If you say this is case closed, then so be it. I won't be arguing about it further.

there-arent-any-issues-if-you-close-them-all

wosym commented 4 years ago

@wosym, you can't pinpoint a connection between this driver and your reset-bug. Other than that bug neither you or I can reproduce the original stated issue, which I therefore closed.

I can not pinpoint the cause of the issue, no. But that's not the point of opening the issue: it's informing others that an issue exists.

I don't expect the driver to be the reason for your reset-bug

Neither do I, like I already said before. This issue merely states that under certain circumstances (or with certain boards), incorrect values can be received

and hence asked you twice to try other tests before suspecting the driver.

What other tests do you expect me to run? I've been using RIOT AVR for almost half a year. Pretty much everything works. Also: a bare metal driver for this sensor works, so if the RIOT driver does not work out of the box (with the same wiring), something is off. Not necessarily something with the driver, but this should not be acceptable behavior.

I have reason for that, 1. it works fine with other boards as you and I both confirmed,

Tests should run on all supported boards, unless there's a valid reason for it (e.g. memory constraints). For an extremely simple sensor like this one, I don't see any valid reasons.

  1. sometimes these kinds of issues just appear on some boards/even board revisions/toolchains/software versions/etc. As long as you can't or won't rule out other reasons I can't help here.

If the issue appears, it's still an issue. Even if the cause is something that has nothing to do with the driver.

If you are eager to fixing this, find a reason for your bug and if it is related to this driver feel free to ping me and I will reopen this issue. If it is not connected, open a separate issue. I don't have the board in question though.

I spent quite a lot of time investigating this issue before I opened the issue. That was back in January. Unfortunately, I no longer have the luxury of spending several hours on a random device that I don't even use. But once again: this issue never said the cause of the issue was the driver. It merely announced the existence of the issue.

miri64 commented 4 years ago

Sorry to step in here, but "Huh??!?" Why should an issue be closed only because the bug can not be pin-pointed exactly? If that would be the case, than don't make an issue, make a PR. Maybe it is the driver mentioned in the issue, maybe its not. Fact is: A misbehavior was noticed in a certain combination of board, device and application and documented in this issue. If there are new insights, that might help to pin-point this bug, title and description of the issue can be edited as such. This way when someone has time and leisure to fix it they have some first pointers and if someone else stumbles over this bug independently they know we know. Otherwise, another issue is just opened. I'm reopening this.

miri64 commented 4 years ago

@wosym please update the original description and title to be in line with the latest findings.

haukepetersen commented 4 years ago

To calm the waves here a little, how about we simply look at the distance variable definition in the driver header file:

typedef struct {
    srf04_params_t p;   /**< GPIO Ports of device */
    int distance;       /**< raw time of flight distance */
    uint32_t time;      /**< timestamp of trigger or echo */
} srf04_t;

It says int, and discussing about an issue on 8-bit hardware, that is quite susceptible :-) So lets look on how this is used:

// srf04.c, line 40:
        dev->distance = (t - dev->time);
// srf04.c line 92:
return ((dev->distance * 100) / SRF04_DISTANCE);

-> if distance is 16-bit value, and it is on avr8-platforms, this would explain the overflows

@wosym would you mind changing the type of the distance field to e.g. int32_t and recheck if your issue does still exist?

wosym commented 4 years ago

@miri64 I updated the Issue-text with the new findings and a new hypothesis. I can't change the title though, I think a maintainer needs to do that.

@haukepetersen I will try to do another test on both the m328p and 1284p somewhere this week.

miri64 commented 4 years ago

@miri64 I updated the Issue-text with the new findings and a new hypothesis. I can't change the title though, I think a maintainer needs to do that.

Like this?

wosym commented 4 years ago

@haukepetersen I did some tests on both boards, but unfortunately I don't have good news.

First I tested the m328p again, without any changes to the code except for the pinconfig. Like before, it showed about 4 good readings and then stopped. But this time, I tried resetting it afterwards (because I remembered seeing odd behavior back in Januari when doing this on the 1284p) . You would expect it to restart the program and show +/- 4 reading again, but no. It starts spamming blank lines infinitely. I recorded a video demonstrating this behavior.

Then I applied your suggested fix (which sounds reasonable). This broke the measurements: The output was "Error: no valid data available". I remember having tried this fix back in december/januari, and having experienced the same results.

Then I reverted the fix and hooked up the 1284p (and also changing the pin configs). The first time I ran it, I had very similar behavior to what happened after I reset the program on the 328p: it spammed infinite blank lines. Except now occasionally there appeared random patterns of the letter 'H'. Every time I pressed the reset button it showed the "This is RIOT" message and "starting srf04 test", so we can rule out a problem with the uart connections.

Then I applied your fix, and it showed the same behavior as on the m328p: "Error: no valid data available". I also made a video of this experiment. Do note that I press the reset button several times in the video. each time it shows the "This is Riot" message.

I tried to also make a video of the 1284p by reverting the fix, but for some reason the behavior remained the same... it did not show the empty lines spam with the letter 'H' again... I tried several times to reproduce this behavior, but I could not.

I remember having seen this back in Januari when experimenting with potential fixes to the problem I was experiencing back then, but can not recall having found a cause.

I have no clue what causes this behavior, and to be honest I also don't know if it is related to the original issue. Somehow it feels like this driver starts writing in illegal memory, causing this unexpected behavior.

haukepetersen commented 4 years ago

Hmm, too bad...

Aside from that I do not have any other idea right now - I have never used RIOT on any ATmega platforms... My only hints would be to look in the following directions:

kaspar030 commented 4 years ago

My only hints would be to look in the following directions:

kaspar030 commented 4 years ago

Then I applied your suggested fix (which sounds reasonable). This broke the measurements: The output was "Error: no valid data available".

Did you fix the type in both srf04.h and in the test main.c (tests/driver_srf04/main.c line 39?

wosym commented 4 years ago

Did you fix the type in both srf04.h and in the test main.c (tests/driver_srf04/main.c line 39?

Can't tell for sure. It's been almost a month ago since I tested it, and I didn't keep the branch. I'll try to redo the test one of these days. But even if it wasn't changed, it wouldn't explain the spammy empty-lines behaviour. Or would it?

kaspar030 commented 4 years ago

I'll try to redo the test one of these days. But even if it wasn't changed, it wouldn't explain the spammy empty-lines behaviour. Or would it?

No it wouldn't. Also, by reading the code, I don't see how this would change things.

Seeing this issue is from January, it was probably branched before the huge xtimer update in 2020.01. Before that, xtimer was not isr-safe on <32bit timers (the atmegas are using 16bit), so with the 50000us sample period, there's a high chance of xtimer's underlying timer to overflow. This could cause issues. But it shouldn't cause the spammy empty-lines behaviour, either. ;)

TL;DR it would make sense to try current master, together with the type fixes.

wosym commented 4 years ago

it would make sense to try current master, together with the type fixes.

The experiment was done on a freshly pulled master.

kaspar030 commented 4 years ago

@haukepetersen @SemjonKerner do you have the sensor at FU?

wosym commented 4 years ago

@kaspar030 I did some more testing (This time again with an 1284p, but a different board. I scavenged the previous one for parts...) What I did first:

What I tried:

2020-08-06 22:42:37,211 # main(): This is RIOT! (Version: 2020.10-devel-600-g95ae7) 2020-08-06 22:42:37,911 # SRF04 range finder example��������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������������

2020-08-06 22:42:37,971 # main(): This is RIOT! (Version: 2020.10-devel-600-g95ae7) 2020-08-06 22:42:38,344 # SRF04 range finder example��������������������������������������������������������������������������������������������������������������

2020-08-06 22:42:38,404 # main(): This is RIOT! (Version: 2020.10-devel-600-g95ae7) 2020-08-06 22:42:38,729 # SRF04 range finder example������������������������������������������������������������������������������������ 2020-08-06 22:42:38,789 # main(): This is RIOT! (Version: 2020.10-devel-600-g95ae7) 2020-08-06 22:42:39,122 # SRF04 range finder example������������������������������������������������������������������������������������

2020-08-06 22:42:39,181 # main(): This is RIOT! (Version: 2020.10-devel-600-g95ae7) 2020-08-06 22:42:39,538 # SRF04 range finder example����������������������������������������������������������������������������������������������

(symbols show up as question marks in diamonds on my terminal and browser, but it might be different with other character sets, I don't know). Strangely enough... this is AGAIN different behavior than before...
(do note: reset was pressed several times in the above log. The amount of symbols it printed was different each time, but didn't continue infinitely)

What I then did was set the ENABLE_DEBUG define in the srf04.c file. The output was similar, but it showed different symbols:

/home/wouter/repo/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyUSB0" -b "9600"
Twisted not available, please install it if you want to use pyterm's JSON capabilities 2020-08-06 22:45:44,478 # Connect to serial port /dev/ttyUSB0 Welcome to pyterm! Type '/exit' to exit. 2020-08-06 22:45:51,548 # ange finder example�HĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊH� 2020-08-06 22:45:51,608 # main(): This is RIOT! (Version: 2020.10-devel-600-g95ae7) 2020-08-06 22:45:52,323 # SRF04 range finder example�HĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊ

2020-08-06 22:45:52,383 # main(): This is RIOT! (Version: 2020.10-devel-600-g95ae7) 2020-08-06 22:45:52,808 # SRF04 range finder example�HĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊ

2020-08-06 22:45:52,868 # main(): This is RIOT! (Version: 2020.10-devel-600-g95ae7) 2020-08-06 22:45:53,288 # SRF04 range finder example�HĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊH� 2020-08-06 22:45:53,347 # main(): This is RIOT! (Version: 2020.10-devel-600-g95ae7) 2020-08-06 22:45:53,746 # SRF04 range finder example�HĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊH� 2020-08-06 22:45:53,806 # main(): This is RIOT! (Version: 2020.10-devel-600-g95ae7) 2020-08-06 22:45:54,031 # SRF04 range finder example�HĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊH�

2020-08-06 22:45:54,091 # main(): This is RIOT! (Version: 2020.10-devel-600-g95ae7) 2020-08-06 22:45:54,526 # SRF04 range finder example�HĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊHĊH� 2020-08-06 22:45:54,585 # main(): This is RIOT! (Version: 2020.10-devel-600-g95ae7)


(Again, reset was pressed every time the output stopped)

(again tested on 2 different srf04 sensors. Behavior didn't change)

I placed a git diff here: https://pastebin.com/w0W7iiKt (to preserve formatting)

@kaspar030 you mentioned increasing stack size. Any suggestions on which one I should increase?
kaspar030 commented 4 years ago

@kaspar030 you mentioned increasing stack size. Any suggestions on which one I should increase?

main and idle would make sense. I think atmega executes isrs on the stack of the active thread, which can be both in this case.

try adding this to the application makefile:

CFLAGS += -DTHREAD_STACKSIZE_DEFAULT=1024 -DTHREAD_STACKSIZE_IDLE=512

(main is derived from default, on atmegas, the default is 512. for idle, it is 128 IIRC.)

wosym commented 4 years ago

Aha, we're getting somewhere!

After adding that to the makefile (with still the previous changes) the behaviour is similar to what I experienced a few weeks ago. I recorded and uploaded the output here. The first 40 seconds are without debug enabled, the last 20 seconds are with debug enabled.

gdiribarne commented 3 years ago

Hello,

I have read all the exchanges. I needed to use sfr04 with a arduino-mega2560 and I have the same issue. I reproduced and fixed the issue. @kaspar030 had a good solution which consists in adapting the int32_t distance inside the sfr04_dev structure.

The main issue is here, since on AVR, it creates an overflow for AVR integer. return ((dev->distance * 100) / SRF04_DISTANCE); However, the function result may return an int since the 2 mm <= distance is <= 4000 mm.

I needed to adapt the test using different pin

If you need, I can manage the pull-request issue and propose a patch (I added also some comments, etc.) which would be my first contrib to the RIOT OS ;-) The resulting tests are conform to expected. I didn't notice any issue with bad reading or so. While reading the code, I had some other ideas to make the srf04 code. I'll propose a new pull request.

Here is the result of the test: main(): This is RIOT! (Version: 2021.01-devel-997-g63af2) SRF04 range finder example D: 2029 mm D: 2016 mm D: 2016 mm D: 2016 mm D: 2030 mm D: 2020 mm D: 2016 mm D: 2030 mm D: 2020 mm D: 2021 mm D: 2029 mm D: 2029 mm D: 2016 mm D: 2020 mm D: 2020 mm D: 2016 mm D: 2015 mm D: 2021 mm D: 2016 mm D: 2016 mm D: 2030 mm D: 189 mm (hand ahead the device - moving a little bit) D: 176 mm D: 153 mm D: 157 mm D: 154 mm D: 152 mm D: 155 mm D: 149 mm D: 168 mm D: 167 mm D: 171 mm D: 144 mm D: 150 mm D: 156 mm D: 152 mm D: 149 mm D: 150 mm D: 158 mm D: 168 mm D: 164 mm D: 169 mm D: 164 mm D: 150 mm D: 145 mm D: 137 mm D: 173 mm D: 141 mm D: 128 mm D: 129 mm D: 121 mm D: 121 mm D: 118 mm D: 114 mm D: 110 mm D: 110 mm D: 105 mm D: 105 mm D: 101 mm D: 93 mm D: 77 mm D: 70 mm D: 73 mm D: 71 mm D: 76 mm D: 69 mm D: 80 mm D: 83 mm D: 92 mm D: 96 mm D: 100 mm D: 108 mm D: 175 mm D: 163 mm D: 153 mm D: 137 mm D: 152 mm D: 140 mm D: 126 mm D: 128 mm D: 134 mm D: 140 mm D: 141 mm D: 145 mm D: 137 mm D: 139 mm D: 143 mm D: 2021 mm (hand exit - returning to measuring something else) D: 2030 mm D: 2021 mm D: 2020 mm ...

wosym commented 3 years ago

Haven't been able to test it myself yet, but I think it's safe to assume #15494 has fixed this issue. So I guess we can go ahead and close this one?