Pages: [1] 2   Go Down
Author Topic: Interesting discovery - Can you figure out what's wrong?  (Read 1263 times)
0 Members and 1 Guest are viewing this topic.
Wellington, New Zealand
Offline Offline
Jr. Member
**
Karma: 0
Posts: 65
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

  String test(char A,int C, String R)
  {
    if(C > 10)
      return A + R;
    else if(C < -10)
      return A + "-" + R;
  }

Notice  anything wrong?

This code causes my controller lock-up...
Logged


Poole, Dorset, UK
Offline Offline
Edison Member
*
Karma: 50
Posts: 2199
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Notice  anything wrong?

Yes you are - read the sticky at the top of the forum.

Quote
This code causes my controller lock-up...

We deal with arduinos here not controllers/pics/pis/ ect.

Mark
Logged

Wellington, New Zealand
Offline Offline
Jr. Member
**
Karma: 0
Posts: 65
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

We deal with arduinos here not controllers/pics/pis/ ect.

I see... So an Arduino's not a Micro controller then?

But thanks for the friendly greetings and advice. smiley-wink

Just want to point out that its worth ensuring all your code paths return values. In this case, the second "if" did not return a value at all times causing my Arduino "Micro Controller" to lock up.
(The IDE also doesn't warn you of this issue)

Exception handling would be nice for this, but too resource intensive for a MCU I take it..
Logged


0
Offline Offline
Shannon Member
****
Karma: 200
Posts: 11672
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Unfortunately warnings are disabled in the Arduino compiler (presumably because it
confuses newcomers to programming), so missing return statements are not reported.
C/C++ does not require compilers to error-out on such broken dataflow - presumably
because of all the existing poorly written C code out there in the real world.
Logged

[ I won't respond to messages, use the forum please ]

Phillipsburg, NJ
Offline Offline
Full Member
***
Karma: 6
Posts: 184
Author: Matrix Keypad Library
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Unfortunately warnings are disabled in the Arduino compiler
More like "disabled by default".  If you go to File->Preferences and check the box for compilation next to Show verbose output during: then hit verify you get the following compiler output:

Blink.ino: In function 'String test(char, int, String)':
Blink.ino:32: warning: control reaches end of non-void function

Quote
(presumably because it confuses newcomers to programming)world.
I like to think that it protects those of us who like to help from programmers and newcomers who might have O.C.D. smiley-wink
Logged

UK
Offline Offline
Shannon Member
****
Karma: 222
Posts: 12534
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Exception handling would be nice for this, but too resource intensive for a MCU I take it..

The Arduino doesn't have any concept of virtual memory management so it will not give you 'segmentation fault' errors and so on if your program accesses an invalid location, as would happen in this case if your function ends without returning a value the caller would then use whatever value happened to be left in the stack return frame. You will get similar problems if you use uninitialised pointers, array index values etc. The compiler used by Arduino is capable of warning about many of these errors but for some strange reason those warnings have been switched off by default.

As an aside, the String class you use in your example can also be problematic since it exposes a heap corruption problem in versions up to 1.0.4 and introduces the risk of heap fragmentation in all versions. In a system with such limited heap space and such limited ability to manage the heap, using dynamic (heap) memory to store dynamic data is not a smart approach. I recommend that you use c-strings (null-terminated char arrays) instead. There are no particular advantages to using the String class and IMO you would be better off forgetting that it exists.
Logged

I only provide help via the forum - please do not contact me for private consultancy.

Wellington, New Zealand
Offline Offline
Jr. Member
**
Karma: 0
Posts: 65
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Thanks for all the info...

I turned on verbose output and it pointed out some useful stuff. Also cleared up a few things I wondered about.

Regarding the String class, I do agree that its not the best way to go. My first thought was actually that using String was
causing the lock-ups.  smiley-grin

Logged


Wellington, New Zealand
Offline Offline
Jr. Member
**
Karma: 0
Posts: 65
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

As an aside, the String class you use in your example can also be problematic since it exposes a heap corruption problem in versions up to 1.0.4 and introduces the risk of heap fragmentation in all versions. In a system with such limited heap space and such limited ability to manage the heap, using dynamic (heap) memory to store dynamic data is not a smart approach. I recommend that you use c-strings (null-terminated char arrays) instead. There are no particular advantages to using the String class and IMO you would be better off forgetting that it exists.

Peter, String has a huge advantages... smiley It prevents a huge amount of frustration.

The function I am working on will generate GCode commands.. I just want to build simple string but I am finding it hard to even just add a char to char *.  smiley-eek-blue
Logged


California
Offline Offline
Faraday Member
**
Karma: 88
Posts: 3356
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Peter, String has a huge advantages... smiley It prevents a huge amount of frustration.
It also causes plenty of frustration as well.
Quote

The function I am working on will generate GCode commands.. I just want to build simple string but I am finding it hard to even just add a char to char *.  smiley-eek-blue
sprintf(), strcat(), etc.
Logged

UK
Offline Offline
Shannon Member
****
Karma: 222
Posts: 12534
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

I am finding it hard to even just add a char to char *.  smiley-eek-blue

Typically it could be done something like this:
Code:
// append c to the string held in buffer
buffer[len++] = c;

// put a null terminator at the new end of the string
buffer[len] = '\0';

You will see plenty of examples of doing this sort of thing if you look for example sketches that read from the serial port.
Logged

I only provide help via the forum - please do not contact me for private consultancy.

Wellington, New Zealand
Offline Offline
Jr. Member
**
Karma: 0
Posts: 65
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

thanks...

Yes.. I noticed that  a lot of people was doing it like that.

If I declare a Char Temp[100] and I insert 50 characters followed by a null terminator. Does that reserve the full 100 in memory or only the bit up to the null terminator?

thanks again for all the help.
Logged


Mid-Atlantic, USA
Offline Offline
God Member
*****
Karma: 30
Posts: 514
"Remember kids, the only difference between Science and screwing around is writing it down." - Adam Savage
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

As an aside, the String class you use in your example can also be problematic since it exposes a heap corruption problem in versions up to 1.0.4 and introduces the risk of heap fragmentation in all versions. In a system with such limited heap space and such limited ability to manage the heap, using dynamic (heap) memory to store dynamic data is not a smart approach. I recommend that you use c-strings (null-terminated char arrays) instead. There are no particular advantages to using the String class and IMO you would be better off forgetting that it exists.

Peter, String has a huge advantages... smiley It prevents a huge amount of frustration.

The function I am working on will generate GCode commands.. I just want to build simple string but I am finding it hard to even just add a char to char *.  smiley-eek-blue

Well, adding a char variable to a char string can be trivial if you know exactly where in the string the char variable goes by index. (Either by having an index variable, or knowing a rigid character position structure.) That's because it's just an array that can be accessed just like any other array. (You as the programmer just need to make sure there is a null (0x00, '\0', 0, 00, etc.) at the index position right after the last character of interest before trying to handle the char string as a singular entity.) If you want to put 'r' at the 10th position of char string outputBuffer, it's as simple as outputBuffer[9] = 'r';. If you are continually adding a new character, simply keep track of where you are in the char string with an index variable. Assign the new char at the current index location, increment the index, and insert a null at the new current index location. Wash, rinse, repeat for each new char to be appended to the char string. (Ok, yes, you also need to watch to make sure you don't try to go past the bounds of your char string...)

The C string libraries just make this easier for you as long as you can remember the abbreviation construction of the function names, or have http://www.cplusplus.com/reference/cstring/ bookmarked. (It's the latter for me...)
Logged


Global Moderator
Offline Offline
Brattain Member
*****
Karma: 474
Posts: 18696
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset


If I declare a Char Temp[100] and I insert 50 characters followed by a null terminator. Does that reserve the full 100 in memory or only the bit up to the null terminator?

It reserves the amount you declare.
Logged

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 601
Posts: 48543
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Typically it could be done something like this
I'm not sure that it is a good idea to give an example that makes any assumptions about what
Quote
I am finding it hard to even just add a char to char *.
means.

I find that people that make statements like this assume that pointers and arrays are the same thing. While there are strong ties between pointers and arrays, that is ONLY true when the pointer points to allocated memory. Far too often, newbies create a pointer that points to nothing, and then assume that they can use it as an array that holds as much as they want.

Rather than suggesting how they might do what they suggest in the question, I think it's better to ask them to post some code, so that it can be determined that the pointer in question does indeed point to allocated memory.
Logged

Wellington, New Zealand
Offline Offline
Jr. Member
**
Karma: 0
Posts: 65
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
  struct position {
    int x;
    int y;
    int z;
    position(){ 0; 0; 0; }
    position(int X,int Y, int Z){ x = X; y = Y; z = Z; }
  } CurrentPosition, DefaultPosition;

  void GRBLCommand()
  {
    int XChange = CurrentPosition.x - DefaultPosition.x;
    int YChange = CurrentPosition.y - DefaultPosition.y;
    int ZChange = CurrentPosition.z - DefaultPosition.z;
   
    if(
        (XChange > 10 || XChange < -10) ||
        (YChange > 10 || YChange < -10) ||
        (ZChange > 10 || ZChange < -10)
      )
    {
      Serial.print("G1 ");
   
      // This can be shortened  to a method that takes the Change,Axis letter and returned the string.
      if(XChange < -10) Serial.print("X-1000");
      if(XChange > 10)  Serial.print("X1000");
     
      if(YChange < -10) Serial.print("Y-1000");
      if(YChange > 10)  Serial.print("Y1000");
     
      if(ZChange < -10) Serial.print("Z-1000");
      if(ZChange > 10)  Serial.print("Z1000");
     
      Serial.println("F100");
    }
    else
    {
      Serial.print("!");
    }
  }

This is my working code. My original idea was to create method that returned a string that gets printed with Serial.print();
I could not get that going because String kept on locking up my Arduino. Because I can't get strings to work properly I ended up just doing the Serial.print inside the method. (It works but I don't like it cause I am sure the code can look much simpler.)

Any ideas on how I can optimize this method and removing the dependency on Serial?

Thanks for everyone's help up to now.


Logged


Pages: [1] 2   Go Up
Jump to: