Safecast / bGeigieNanoKit

bGeigieNano is a kit version of the bGeigie mobile survey geiger counter designed to fit into a Pelican Micro Case 1010.
https://safecast.org/devices/bgeigie-nano/
113 stars 43 forks source link

Code correction needed? - bGeigie-nano.ino missing two braces at end of Main loop. #49

Open dbgitter opened 5 years ago

dbgitter commented 5 years ago

Reviewing the code revealed that first and second braces for Main loop were not matched (using ultraedit ctrl-B to match braces). Adding two right braces at the end of the loop allowed all braces to match.

fakufaku commented 5 years ago

Hi @dbgitter , thanks for pointing this out. This is very puzzling. Indeed, it seems that if two braces are missing it shouldn't compile at all.

The confusion with the braces matching might be due to the large number of #if/#endif statements throughout the code. I think the outer brace of the (Main) loop (line 611) is matched with the opening brace of the for loop line 453. However, the matching closing brace of this for loop is in fact on line 477. Within this for loop, the following piece of code

 460 #if ENABLE_SOFTGPS
 461     while (gpsSerial.available())
 462     {
 463       char c = gpsSerial.read();
 464 #else
 465     while (Serial.available())
 466     {
 467       char c = Serial.read();
 468 #endif

adds two opening braces, but only one of these is used at compile time due to the pre-compiler options. This might be what throws off the parenthesis matching algorithm of the text editor.

dbgitter commented 5 years ago

Sorry, I’m not familiar with the details of conditional compile statements as used here, but I would think the braces should go in and out of the code in pairs. And because no one really ever knows exactly how a compiler may be affected by errors this may cause a seemingly random error every once in a while when a block of code is added or removed.

On Sep 25, 2018, at 2:29 AM, Robin Scheibler notifications@github.com wrote:

Hi @dbgitter https://github.com/dbgitter , thanks for pointing this out. This is very puzzling. Indeed, it seems if two braces are missing it shouldn't compile at all.

The confusion with the braces matching might be due to the large number of #if/#endif statements throughout the code. I think the outer brace of the (Main) loop (line 611) is matched with the opening brace of the for loop line 453. However, the matching closing brace of this for loop is in fact on line 477. Within this for loop, the following piece of code

460 #if ENABLE_SOFTGPS 461 while (gpsSerial.available()) 462 { 463 char c = gpsSerial.read(); 464 #else 465 while (Serial.available()) 466 { 467 char c = Serial.read(); 468 #endif adds two opening braces, but only one of these is used at compile time due to the pre-compiler options. This might be what throws off the parenthesis matching algorithm of the text editor.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Safecast/bGeigieNanoKit/issues/49#issuecomment-424222434, or mute the thread https://github.com/notifications/unsubscribe-auth/AbqZgINY6lTVAsGKStqdeDGfD4QNQvoPks5uec1CgaJpZM4W36Oz.

robouden commented 5 years ago

There is are some very long routines with if in there. If as Robin mentioned the options are selected in the config file. It all should be fine. Compiling would be stopped when it was not correctly close.

regards rob

Regards, Rob Oudendijk Yuka Hayashi http://yr-design.biz http://oudendijk.biz http://about.me/robouden tel +81 80-22605966 Skype: robouden Facebook:robouden http://on.fb.me/QeKw2P linkedin:robouden https://www.linkedin.com/in/roboudendijk

On Tue, Sep 25, 2018 at 3:41 PM dbgitter notifications@github.com wrote:

Sorry, I’m not familiar with the details of conditional compile statements as used here, but I would think the braces should go in and out of the code in pairs. And because no one really ever knows exactly how a compiler may be affected by errors this may cause a seemingly random error every once in a while when a block of code is added or removed.

On Sep 25, 2018, at 2:29 AM, Robin Scheibler notifications@github.com wrote:

Hi @dbgitter https://github.com/dbgitter , thanks for pointing this out. This is very puzzling. Indeed, it seems if two braces are missing it shouldn't compile at all.

The confusion with the braces matching might be due to the large number of #if/#endif statements throughout the code. I think the outer brace of the (Main) loop (line 611) is matched with the opening brace of the for loop line 453. However, the matching closing brace of this for loop is in fact on line 477. Within this for loop, the following piece of code

460 #if ENABLE_SOFTGPS 461 while (gpsSerial.available()) 462 { 463 char c = gpsSerial.read(); 464 #else 465 while (Serial.available()) 466 { 467 char c = Serial.read(); 468 #endif adds two opening braces, but only one of these is used at compile time due to the pre-compiler options. This might be what throws off the parenthesis matching algorithm of the text editor.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/Safecast/bGeigieNanoKit/issues/49#issuecomment-424222434>, or mute the thread < https://github.com/notifications/unsubscribe-auth/AbqZgINY6lTVAsGKStqdeDGfD4QNQvoPks5uec1CgaJpZM4W36Oz .

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Safecast/bGeigieNanoKit/issues/49#issuecomment-424225083, or mute the thread https://github.com/notifications/unsubscribe-auth/ABUqtqTy0v8s9vmdkCxBRCGSN-7dcEgwks5uedAagaJpZM4W36Oz .

fakufaku commented 5 years ago

@dbgitter Sorry, I made a small terminology mistake, this is not a precompiler, but a preprocessor ("The" C preprocessor). It will parse the code and do some automatic modifications based on instructions starting with a # (#define, #include, etc). In the case of #if/#else statement, either set of instructions will be removed prior to compilation. At compile time, all braces are thus matching in the above code statement.

As Rob emphasized, were some parenthesis missing, the compiler should fail with some error. Was it the case for you ?

The above code fragment provides evidence for one extra opening brace, and I'm pretty sure that if we look close enough, we can find the second one.

dbgitter commented 5 years ago

@robouden Attached is a snippet of the Main loop with closing comments added for all the conditional blocks. Could you please have a look and comment on any that are wrong? Thank you. mainloopdbgitter.txt

fakufaku commented 5 years ago

@dbgitter That makes the code much easier to read! Thanks for doing that. I checked the file you provided and it seemed correct to me.

robouden commented 5 years ago

@dbgitter thanks. Yes, much nicer. Good job!!

dbgitter commented 5 years ago

@robouden @fakufaku You are most welcome! My original intent was to try to make a small version of the code to load into an Arduino NANO that I could program with the Arduino IDE so I could troubleshoot my bgeigie nano kit #3106. The kit works and Safecast data was uploaded, but the display is intermittent. Do either of you know who might be able to help with signal tracing?

robouden commented 5 years ago

@bdgitter... I made much smaller versions with using ASCII only versions. That leaves about 7K over and complies fine in Arduino IDE 1.8.5. It also allows for full 128x64 use of the SSD1306. Also allows you to add other sensors like gas/temp/humidity/shocks sensors etc. Let me know me know if you want to have the code.

regards rob

Regards, Rob Oudendijk Yuka Hayashi http://yr-design.biz http://oudendijk.biz http://about.me/robouden tel +81 80-22605966 Skype: robouden Facebook:robouden http://on.fb.me/QeKw2P linkedin:robouden https://www.linkedin.com/in/roboudendijk

On Fri, Sep 28, 2018 at 2:42 AM dbgitter notifications@github.com wrote:

@robouden https://github.com/robouden @fakufaku https://github.com/fakufaku You are most welcome! My original intent was to try to make a small version of the code to load into an Arduino NANO that I could program with the Arduino IDE so I could troubleshoot my bgeigie nano kit #3106. The kit works and Safecast data was uploaded, but the display is intermittent. Do either of you know who might be able to help with signal tracing?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Safecast/bGeigieNanoKit/issues/49#issuecomment-425180846, or mute the thread https://github.com/notifications/unsubscribe-auth/ABUqtlGCr09dPwLSP0O5met-jPzu57t7ks5ufQ3ggaJpZM4W36Oz .

dbgitter commented 5 years ago

@robouden Yes that would be very helpful to have a smaller scale set of software that works with the Arduino IDE.
Thank you.

Dan

robouden commented 5 years ago

https://github.com/Safecast/bGeigieNanoKit/tree/ASCII and also on my private repo for first generation board at: https://github.com/robouden/bGiegeiNano_rob

rob