Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stack overflow #2

Open
dvtomas opened this issue Sep 12, 2018 · 8 comments
Open

Stack overflow #2

dvtomas opened this issue Sep 12, 2018 · 8 comments
Assignees

Comments

@dvtomas
Copy link

dvtomas commented Sep 12, 2018

Hi,
nmea 0.0.7 chokes with Stack Overflow on this data dumped from the GPS integrated into the Algiz tablet:

$GNVTG,,,,,,,,,N*2E
$GNGNS,181604.00,,,,,NN,00,99.99,,,,*59
$GNGGA,181604.00,,,,,0,00,99.99,,,,,,*72
$GNGSA,A,1,,,,,,,,,,,,,99.99,99.99,99.99*2E
$GNGSA,A,1,,,,,,,,,,,,,99.99,99.99,99.99*2E
$GPGSV,4,1,13,02,35,291,,03,09,129,,05,14,305,,06,38,226,*7D
$GPGSV,4,2,13,07,56,177,,09,70,067,,16,20,055,,23,41,076,*77
$GPGSV,4,3,13,26,10,030,,29,05,341,,30,26,199,,36,30,158,*7E
$GPGSV,4,4,13,49,32,192,*4D
$GLGSV,3,1,10,66,45,091,,67,67,334,,68,17,297,,75,13,025,*68
$GLGSV,3,2,10,76,49,059,,77,40,156,,78,00,183,,82,15,246,*68
$GLGSV,3,3,10,83,28,298,,84,10,352,*6F
$GNGLL,,,,,181604.00,V,N*5E
$GNGRS,181604.00,1,,,,,,,,,,,,*5A
$GNGRS,181604.00,1,,,,,,,,,,,,*5A
$GNGST,181604.00,0.0000,,,,5773795,5773795,5773794*4F
$GNZDA,181604.00,12,09,2018,00,00*73
$GNGBS,181604.00,,,,,,,*7B
$GNTXT,01,01,02,u-blox AG - www.u-blox.com*4E
$GNTXT,01,01,02,HW UBX-M8030 00080000*60
$GNTXT,01,01,02,EXT CORE 3.01 (107900)*33
$GNTXT,01,01,02,ROM BASE 3.01 (107888)*25
$GNTXT,01,01,02,FWVER=SPG 3.01*46
$GNTXT,01,01,02,PROTVER=18.00*11
$GNTXT,01,01,02,MOD=NEO-M8N-0*67
$GNTXT,01,01,02,FIS=0xC22536 (100111)*28
$GNTXT,01,01,02,GPS;GLO;GAL;BDS*77
$GNTXT,01,01,02,SBAS;IMES;QZSS*49
$GNTXT,01,01,02,GNSS OTP=GPS;GLO*37
$GNTXT,01,01,02,LLC=FFFFFFFF-FFFFFFED-FFFFFFFF-FFFFFFFF-FFFFFF69*23
$GNTXT,01,01,02,ANTSUPERV=AC SD PDoS SR*3E
$GNTXT,01,01,02,ANTSTATUS=OK*25
$GNTXT,01,01,02,PF=3FF*4B
$GNVTG,,,,,,,,,N*2E
$GNGNS,181605.00,,,,,NN,00,99.99,,,,*58
$GNGGA,181605.00,,,,,0,00,99.99,,,,,,*73
@Dushistov Dushistov self-assigned this Sep 13, 2018
@Dushistov
Copy link
Collaborator

Hi @dvtomas, with your nmea I got such error "Unknown or implemented sentence type: GNS at line 2",
but no "Stack Overflow". This is also not good, but stack overflow is much more worse.

Can you provide some details, like type of CPU, version of rustc and stack trace?

@dvtomas
Copy link
Author

dvtomas commented Sep 14, 2018

Hi,
I've created a simple repository exhibiting the problem for you here: https://github.com/dvtomas/nmea-fail.

cargo run gives me

buffer = "\n"
buffer = "$GNGGA,181604.00,llll.lll,a,yyyy.yyy,a,0,00,99.99,w.w,M,x.x,M,,*72\n"

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
[1]    13633 abort      cargo run
cargo 1.27.0 (1e95190e5 2018-05-27)
rustc 1.27.0 (3eda71b00 2018-06-19)

If you need more details, please just ask me.

@Dushistov
Copy link
Collaborator

Hi @dvtomas , thank you for the way to reproduce problem.
The problem was in println!("{:?}", nmea);, debug implementation call it self.

Dushistov added a commit that referenced this issue Sep 17, 2018
@Dushistov
Copy link
Collaborator

@dvtomas, I fix it in master. I plan to add support for unknown messages in your log,
and only after that close the issue and release new version.

@dvtomas
Copy link
Author

dvtomas commented Sep 18, 2018

Ok, I just tried latest master and it seems to work fine. Thank you for your effort. I see you have pasted my code as a test. I think it would be better to remove the println and eprintln statements (ad262f2#diff-e2a8b794872705e1e70d1fbbf4ddd028R48 and ad262f2#diff-e2a8b794872705e1e70d1fbbf4ddd028R52) so not to clutter the test output. If you want to assert creating the debug string doesn't recurse indefinitely, I think just performing a format!("{:?}, ...) should be enough.

@Dushistov
Copy link
Collaborator

see you have pasted my code as a test. I think it would be better to remove the println and eprintln statements
I think just performing a format!("{:?}, ...) should be enough

From maintainer point of view *print*! in the test is the good thing (of course if it is not benchmark like test),
if all ok cargo test suppresses output, so you don't see anything, but if something wrong you will see not only which assert failed, but also some content, and if your print statements good enough, you almost never need debugging, just go and fix problem.

@dvtomas
Copy link
Author

dvtomas commented Sep 18, 2018

Hmm, my cargo test doesn't supress the output, instead it prints stuff like

running 2 tests
test lps_measuring_context::tests::test_proto_last_signal_data ... ok
test lps_measuring_context::tests::test_process_track_event ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

If this was interspersed with loads of print! output, I would have a hard time checking which tests went ok and which not.

When I need more helpful assertions, I use the extended assert! version:

#[test]
fn test_assert() {
    let x = 1;
    let y = 2;
    assert!(x == y, "X and Y don't equal, x: {}, y: {}", x, y);
}

outputs on cargo test

thread 'gps_nmea::test_assert' panicked at 'X and Y don't equal, x: 1, y: 2', hw_gps/src/gps_nmea.rs:102:5

That is not to say I want to impose my way of working on you, I understand you may like yours more. I just wanted to share what works well for me. As a user, I'm happy with rust-nmea the way it is now.

@dvtomas
Copy link
Author

dvtomas commented Sep 18, 2018

One more tip: A couple of months ago I discovered the pretty-assertions crate, that's been very helpful to me! I'm not sure if you're aware of it.

elpiel pushed a commit that referenced this issue Jul 17, 2022
functional_tests - improve readability and use vec & no_std attribute
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants