Have you actually checked your available memory. You may well be running out of sRam.
Can you confirm that the code as currently found on GitHub also exhibits these characteristics (described in the OP)? I personally don't see anything wrong with the way you have written the "if" (except readability).
I don't have all those libraries so I can't get a clean compile, but I suggest you do one both with and without the braces and compare the object files.
See Gammon Forum : Electronics : Microprocessors : Arduino programming traps, tips and style guide for how to disassemble the resulting .elf files.
Since adding the braces shouldn't make any difference I am guessing that you have overwritten memory somewhere, and that the braces have made the compiler make a slightly different register allocation, this different register allocation is springing a crash caused by the memory leak / overwriting.
How big can the (contents of the) variable iwrap_autocall_index get? (eg. 42)
It is what your code looked like was happening, as I mentioned, the function above it set index 12 to zero and commented on no name. And it seemed like you had used index 13 by accident. Ah well.
The repo code doesn't explicitly set the pointers to NULL, though I did modify line 133 in the "support_bluetooth2_iwrap.h" file to do this as a test:
You can not initialize variables in a header file. What that did was make the function a pure virtual function. Which means that, if the code compiled, some other class inherits from the class you modified, AND actually implements the function. So, you are NOT looking at the code that actually gets executed.
Thanks, everyone, for all of your comments so far. At Paul Stoffregen's recommendation I have ordered an AVR One and will see if what kind of debugging I can manage with Atmel Studio. (I actually found a used device available for a steal on eBay from a local US seller, so here's to hoping...)
KenF:
Have you actually checked your available memory. You may well be running out of sRam.
I haven't actually checked, no. I'll look into the best way to do this on a Teensy++ (I've seen some examples on the Arduino Playground, but haven't verified if they work as-is on this device). I suspect this is not to blame since the compiler projects RAM usage of less than 20% of what is available, and I do not have many large allocations at runtime. I'm acutely aware of the limited memory available and try whenever possible to use flash for constant data like string literals. Buffers malloc()'d at runtime are kept few and small, and free()'d in all cases to avoid memory leaks (assuming I haven't missed any, which is not impossible). But regardless, I will definitely test this to make sure.
Yes, the code on the repository right now exhibits the problem. Thanks for the pointer to object file comparison and .elf investigation; I was not aware of this approach before. I'll read up on this and see what I can do with it.
Depending on the number of paired devices, this can be between 0 and 15. In most applications, it will not go above perhaps 2 or 3. In my test case here, it does not go above 0 because there are no paired devices. The autocall algorithm never kicks in since iwrap_pairings equals 0, and the condition on line 566 won't pass.
PaulS:
You can not initialize variables in a header file. What that did was make the function a pure virtual function. Which means that, if the code compiled, some other class inherits from the class you modified, AND actually implements the function. So, you are NOT looking at the code that actually gets executed.
Maybe I am misunderstanding something about the Arduino build environment, but doesn't this only apply when you are using classes? All of the functions and function pointers involved in this are not part of any class definitions. The kg_evt_bluetooth_inquiry_response function pointer is a global variable, and the my_iwrap_evt_name function is a global function, not a member of a class. Unless the Arduino IDE is wrapping everything in its own giant class, I think it is still allowed to declare and initialize this kind of value in a header file, right?
I think it is still allowed to declare and initialize this kind of value in a header file, right?
You have so many files and such useless comments that it's hard to keep track of what's what.
If the function is not in a class, then nothing I said about it applies.
PaulS:
You have so many files and such useless comments that it's hard to keep track of what's what.
Honestly? That's a bit harsh. I won't disagree on the project complexity alone though. Here's a map:
http://www.keyglove.net/docs/arduino/support__bluetooth2__iwrap_8h.html
http://www.keyglove.net/docs/arduino/files.html
PaulS:
If the function is not in a class, then nothing I said about it applies.
Good to know.
I will continue investigating.
I sussed out your approach which is a bit unusual, but in itself I don't think would cause a problem*. You have a lot of code in .h files which is a bit of a novel approach, however since they are only included once, I don't think would be the issue.
More conventionally, you would have a lot of .cpp files, and manage calls from one to the other by using function prototypes in a .h file.
What device are you running this on? A Teensy++?
Is that the one with a AT90USB1286 processor? 8192 bytes of RAM, 130048 bytes of flash?
I seem to recall some issues with the Atmega2560 where code (or data?) crosses a 64 kB boundary.
- Having said that, your .h file approach effectively produces a monolithic object file which may possibly have problems compared to separate compilation units.
I agree, this is an unusual project structure. I originally took that approach because I ran into issues trying to get the Arduino IDE to compile multiple .cpp/.h files in the same sketch, way back when I started. Even though I managed to get it to compile and work generally, it has created more than a few challenges concerning #include order and which files can reference which other files at which points. At the time, I chalked it up to the way Arduino combined sketch files. I started in 2010 with this, long before v1.0 of the IDE, and maybe things have changed since--or maybe I just had no idea what I was doing when I started, which is more likely, and it always would have worked if I did it the right way.
In either case, if it is possible to use the standard approach of .cpp/.h file combinations today, I will absolutely put in the effort needed to restructure the project.
The monolithic object file is yet another thing that I had not considered, and another reason to make the above change. Thanks for the insight here.
Yes, it's the Teensy++ 2.0 from PJRC, the AT90USB1286 variant.
Yes, it's the Teensy++ 2.0 from PJRC (thanks PaulS)
Paul Stoffregen, the maker of the teensy, posts under his complete name. I do not. You've got the wrong Paul S. there.
PaulS:
Paul Stoffregen, the maker of the teensy, posts under his complete name. I do not. You've got the wrong Paul S. there.
Ah. Oops. Sorry about that.
I will definitely read through that prior to refactoring this project into a more normal format. Thanks!
^^^ EDIT: Good grief, this is fantastically simple. Amazing.
Follow-up on SRAM usage: according to Adafruit's quick RAM check function, I still have over 6kB of ram free out of the 8kB on the chip immediately prior to the offending lines of code, so it doesn't look like I'm running out of memory. That's what I thought, but all the same...good.
I am nowhere near the guru that many on here are, and what this reminds me of is something that I encountered about 30 years ago working on assembler on an IBM mainframe, but when a line of code stops or starts mis-behaving when another line of code is inserted (even a single blank assembly instruction), it would seem to me that there is some kind of memory bounds issue involved.
Not necessarily that there is not enough memory, but that one variable ends up writing over another variable.
Without comparing the decompiled code of the two versions (with and without braces), it would be impossible to figure out why. And even having the decompiled code would be extremely difficult.
Can you possibly re-order the sequence of functions within a portion of the code, so that this function is earlier by one or two functions, or later by one or two functions. If it continues to fail, you probably have something, somewhere, that is setting the pointer to that function (in the if statement), but setting it to something that does not act as a function, and thus it "hangs" or terminates in such a way as to mess things up. (If it doesn't continue to fail, then the code will probably fail elsewhere after having moved the function).
My guess, is that when you put the braces in, you have moved the problem to elsewhere within your code, because somewhere you are overwriting a bit of memory outside of where you think you are writing.
Quick update on this issue: I finished rearranging the code into standard C project form, rather than one .ino and a million .h files. This has greatly simplified the code organization, especially regarding order-of-include challenges that I previously had a hard time solving.
I also noticed that at some point, my Arduino IDE had been set to run the Teensy++ at 16MHz (IDE default), rather than 8MHz. It's been modified with the 3.3v LDO, and per Paul Stoffregen's recommendation, that's a bad idea. An AT90USB1286 @ 3.3v is technically overclocked at 16MHz. Maybe the setting got reset when I updated to v1.0.6 of the IDE instead of the v1.0.5-r2 that I had been using...I really don't know how long it has been that way, but I put it back to 8MHz in the middle of the code rearrangement process.
The API event no longer freezes like it did before, despite not having some of the extra-safe adjustments to the code (braces around the conditional block, explicit NULL tests in "if" conditions, etc.). So far, things appear to be 100% reliable again.
I cannot say for sure what change fixed this behavior since there were many of them simultaneously, and I'm also not sure whether the problem is actually gone or only moved. It could still be an invalid memory dereference operation or buffer overflow that is once again invisible. In case it shows up again. I now have an AVR One debugger to attack the problem with. For the moment though, this case is closed. Thanks for all of your help and insight!
Glad to help, and good to hear your code is better organized. 