Go Down

Topic: IDE 1.6.0 is now available for download (Read 38979 times) previous topic - next topic

Peter_n

#60
Feb 14, 2015, 01:38 pm Last Edit: Feb 14, 2015, 01:38 pm by Peter_n
The array deviceAction[] has now a fixed size of just one element, yet you use more.

pieman

The array deviceAction[] has now a fixed size of just one element, yet you use more.
Ok will make changes but did anything change for arrays between 1.0.6 and 1.6.0?
Look up our website url on our profile page to find out more about the Arduino based Combined RF Boiler Control and RF Learner.

Peter_n

#62
Feb 14, 2015, 01:56 pm Last Edit: Feb 14, 2015, 01:57 pm by Peter_n
Yes, a few changes to the Serial library, perhaps a new compiler, new serial on the computer.
But you have to give a good test sketch, or I don't believe that something is wrong...  :smiley-mr-green:

pieman

Ok will make changes but did anything change for arrays between 1.0.6 and 1.6.0?
Changing the first line from int deviceAction[]={0}; to int deviceAction[200]; has fixed the issue. Needs to be as high as 200 despite the hit to RAM. Thanks guys.
Look up our website url on our profile page to find out more about the Arduino based Combined RF Boiler Control and RF Learner.

ShapeShifter

Ok will make changes but did anything change for arrays between 1.0.6 and 1.6.0?
You were overrunning an array. Once you do that, all bets are off. The question isn't why doesn't it work in 1.6.0, but why did it ever work in 1.0.6?

Why the change? Something may have changed in the way the compiler orders data in memory. Perhaps 1.0.6 put the array at the end of memory, so that it overran into unused space and didn't cause a problem? And 1.6.0 is putting them in another order so setting other data is overwriting data in your array?

You got lucky that it ever worked. Rather than dwell on the differences, I would just be thankful that you found and fixed a very significant bug in your code.

ShapeShifter

Needs to be as high as 200 despite the hit to RAM.
Does it really need to be that high? It looks like buffSize is used as the dimension for the number of characters in inputBuffer and messageFromPC. For each of the numbers going to into deviceAction[], you are consuming multiple incoming characters (one for each digit, and the comma separator.) So the number of actual numbers will be less than buffSize.

What's the maximum count of numbers (between commas) that you can have (not the maximum number of characters)? That's the size of the dimension.

And if you are concerned about RAM, there is the 200 byte array messageFromPC that is declared and never used. There is also the 200 character inputBuffer that is filled with the incoming message, but never used. NULL terminating inputBuffer when an endMarker is received can simply be eliminated. Then, the other block of code that sets input buffer can be simplified to:
Code: [Select]
   if(readInProgress) {
      if(isDigit(x)){
        inString += x;
        deviceAction[rfsignal]=inString.toInt();
      }                              

This will let you delete the declarations for messageFromPC and inputBuffer, saving you 400 bytes. Since you would no longer be indexing into inputBuffer, bytesRecvd (and all the code to maintain it) is also no longer needed.

And while we're on the subject of array overflows, you have no code in place to prevent deviceAction[] from overflowing if more than 200 comma separated values are received. You should add some logic to prevent rfsignal from becoming larger than the deviceAction[] array bounds.

Now that you have some working code, and you've made it through the early development stage where you've tried a bunch of things. it wouldn't be a bad idea to go through and do a cleanup pass. Look for variables that are defined or set and not actually used and get rid of them. Look for blocks of code that are not doing anything useful and get rid of them. Clean out old comments and make sure existing ones are useful and correct. You can probably make the sketch half the size that it is, and still do the same processing. Now that you can properly read and echo the received data, I guess you're going to add some more code to actually do something with that data. If so, cleaning up what you have before taking the next step is probably a very good idea.

pieman

Thanks for taking the time to look through the code ShapeShifter and you are right that I was lucky that my bug wasn't picked up in 1.0.6.

The full sketch already sends the RF signals to RF devices around the home and it works very well, despite the bug.

Most RF signals are well below 100 high / low transitions (200 signals in total), typically being around 26 highs and lows (52 signals) but we need to cover all possibilities. So the array size needs to be 200, in actual fact 202 because there are 2 other important bits of data passed to the Arduino at the same time.

I will strip out the code that is no longer required and add an error check for buffer overflow. The buffer for 200 signals will need to be around 800 (3 characters for the maximum signal size plus the comma separator per signal).

May I ask can I create a 'dynamic' array?

The very first byte sent to the Arduino is the length of the signal to follow. Ideally what I would like to have is a fixed 1 byte array to pick up signal length and then a dynamic array based on this signal length.

 
Look up our website url on our profile page to find out more about the Arduino based Combined RF Boiler Control and RF Learner.

ShapeShifter

The buffer for 200 signals will need to be around 800 (3 characters for the maximum signal size plus the comma separator per signal).
But do you really need to save all of the characters? Is there some other code that you aren't showing that actually uses inputBuffer, bytesRecvd, or messageFromPC? If not, then all you need is the 202 element deviceActions[] array.

Quote
May I ask can I create a 'dynamic' array?
You can create dynamic arrays, but I wouldn't recommend it. You would have to malloc() the array on each incoming message, and free() it after you are done with it. If not done with extreme care, it can cause memory leaks, heap fragmentation, and data corruption. There would have to be a really good reason to use dynamic memory allocation in an embedded application, and in over 30 years of doing embedded programming, I haven't seen it yet.

Unless you have other large data structures that can't coexist with your deviceActions[] array (and don't have to be around at the same time as deciceActions[]) I would stick with a normal static allocation.

You're already implicitly using dynamic memory allocation with your use of the String class to build your incoming numeric value. Some would argue that's already a bad idea.

Quote
The very first byte sent to the Arduino is the length of the signal to follow. Ideally what I would like to have is a fixed 1 byte array to pick up signal length and then a dynamic array based on this signal length.
If it's a fixed single element array, it doesn't need to be an array. Doing so just adds code and computational overhead. Just define it as a simple variable.

That said, given how your code right now looks for delimiters and fills an array with values, putting that first value in a different array or variable just introduces more code and overhead in your parsing routine. When storing the converted value away, you have to look to see if you are at the first value and store that one specially, then adjust the array index to start filling the first element of your array with your second input value. If you were using a state machine to handle the incoming data values, that might make sense. But for this simple case I'd keep it the way you have it and just remember that the first array element has a special meaning.

loeezy

Dear Sir,
Thank you for responding to me, an ignorant person! I have not gotten to learn what a "yun" is or how to use it. I'm only on the 48th page in the book, "Getting Started...."

Here's the message:
.
avrdude: ser_open(): can't open device "COM1": No such file or directory
ioctl("TIOCMGET"): Inappropriate ioctl for device
Problem uploading to board.  See http://www.arduino.cc/en/Guide/Troubleshooting#upload for suggestions.

cmaglie

avrdude: ser_open(): can't open device "COM1": No such file or directory
ioctl("TIOCMGET"): Inappropriate ioctl for device
Problem uploading to board.  See http://www.arduino.cc/en/Guide/Troubleshooting#upload for suggestions.
Hi loeezy,

please check that you have the correct board and port selected (menu Tools->Board and Tools->Port).


C.

ShapeShifter

avrdude: ser_open(): can't open device "COM1": No such file or directory
COM1 is almost never the correct serial port to use.

What other COM ports are listed on the Port menu? The one you want is likely to be a significantly higher number.

Peter_n

loeezy, you had better started a new topic.
Do you have an official Arduino Uno board ? Or a Arduino Yun board ?

If you start a new topic and want me to look at it, send me a Personel Message with a link to that topic.

I prefer not to read a book, but start using the Arduino and learn along the way ;)
You start with the "Blink" example.

pieman

Thanks again ShapeShifter.
I think I had read elsewhere that dynamic arrays are not advised so based on your experience I will dismiss this idea. I currently have 'plenty' of RAM so not really an issue. Removed all the unused variables and traps at a maximum of 200 signals.
Look up our website url on our profile page to find out more about the Arduino based Combined RF Boiler Control and RF Learner.

loeezy

Thanks you-all!  I actually didn't have a port selected for the new version.  It seems to be working well now.  I do have the official arduino uno board and am following the the book "Get Started in Arduino", but don't yet know anything about the yun board....

Thanks again!

ShapeShifter

but don't yet know anything about the yun board....
Be aware that there is a forum section specific to the Yun. Be sure to check it out, especially the sticky FAQ topic at the top of that forum.

Being that the Yun is rather new, a lot of books probably don't mention it. Be aware that the Yun is very much like a Leonardo, which has some significant differences from an Uno, particularly in the way the serial ports and PWM are handled. The Yun also has some special power considerations.

Go Up