One particular weakness with the library has been in lack of error detectection and overall consistent error handling. Often errors are simply ignored. This is one area which needs definite improvement. The motorobots library did a much better job in that area.
Another assumption (with no error checking) is many objects assume only one or two will be declared. For instance, the Waypoint_Driver, etc. Also, because the object needs to communicate with the "C" interrupt routine, I declare a global pointer to the object which will get assigned only to the last one declared if more than one is declared.
Many of the design limitations though are based upon limitations of the current linker script, compiler, tools, and lack of time to investigate further.
Using the singleton's internal static storage method, produced a "mk68k-elf-ld: section data [xxxx-> xxxx] overlaps section .gnu.linkonce.t.__"...thus the singleton is internally just single (static) data rather than single class instance, but I don't think this has much penalty (am I wrong). The following have been problems encountered thus far:
The following design considerations are now standards that I now (at least try) adhere to. Application of these rules will become a necessity beyond a certain number of lines of code; otherwise, you will find yourself endlessly tracing down unforeseen bugs.
Or more specifically returning pointers to local data--slight difference, but legal to return local pointer to global data.
Usually mixing types in return values (boolean/char*) like this require more care and probably should not be done.
char * D_editor::Internal_Font(char * ichar)
{
if ( (*IWchar).font->Type() == D_INTERNAL)
{
i = (*IWchar).uc.Unicode();
ichar[0] = (i & 0x00FF);
ichar[1] = 0;
return ichar;
}
else
return 0; // <--BAD!!! changes global pointer if called 'tptr = internal_font(tptr);'
// should have called 'tptr2 = internal_font(tptr);' where tptr is char [n] and tptr2 is char*
}
NOTE: What you should do to _ALL_ pointers you pass in, is declare: char * D_editor::Internal_Font(char * ichar const), Also, declared char * where appropriate should be declared char* const uc = new (char) [n];', this way you cannot accidentally change the pointer, only the data.
NOTE: This one may be only 68332 as the Windows version executed the same code sucesfully although to be portable it should be considered in any design.
NOTE: This in general is probably not the best solution. If starting from scratch, design an architecture that fills in a generic byte string rather than use structure or structure conversions.
For lack of a better name, when converting a character byte string into a variable length struct; i.e., converting the pHB below, note that the starting address of HB must be word aligned. This is tricky in that in C there isn't a way of guaranteeing a byte alignment in the program--is there?
bool gConnectButton = false; bool RTOS_ON = false; short gSentMessage = false; // Heartbeat message char HB[JAUS_DECLARATION_LENGTH]; jaus_message* pHB= (jaus_message*)HB;
WARNING, Swapping the previous bool as follows:
bool gConnectButton = false; short gSentMessage = false; bool RTOS_ON = false; // Heartbeat message char HB[JAUS_DECLARATION_LENGTH]; jaus_message* pHB= (jaus_message*)HB;
Causes a crash of:
Booting from address $90010 Exception: Address Error Format/Vector=C00C SSW=0215 Fault Addr.=00005377 Data=00004202 Cur. PC=00091FF0 Cnt. Reg.=0008 PC =00091FF0 SR =2200=TR:OFF_S_2_..... VBR =00000000 SFC =5=SD DFC =5=SD USP =0000FC00 SSP* =0007FB10 D0 =00000000 D1 =00004200 D2 =00000000 D3 =00000001 D4 =00000002 D5 =00000029 D6 =00000001 D7 =00000001 A0 =00005375 A1 =00000029 A2 =00000001 A3 =00000000 A4 =00000000 A5 =00000000 A6 =0007FB58 A7 =0007FB10 00091FF0 206E0008 MOVEA.L $8(A6),A0
Note the instruction that caused the crash was pHB->header(...). I'm assuming that the declaration of a short is automatically properly word aligned--the bool then fills the even address and HB starts on an odd address.
When subclassing, watch when you're base class has a method of the same name as another class you're containing. The only error you get is a "syntax error before ';'" on the second use which usually implies it doesn't know about the class!
NOTE: after so many classes it does become difficult to come up with unique names; thus the ubiquitous appendages people often insert like FL_model etc. Although it is difficult with public methods that are supposed to representative.
This is a design methodology I learned is especially useful when designing for portability. The basic idea is you separate all graphical interface elements into a interface class. Theoretically, all calls to the graphics library are isolated into this interface class. The main benefit is apparent--if you decide to switch to another graphical library, you simply have one class to port. Obviously, in very graphically intensive applications this may seem awkward, but I believe the payoff is still worth it. For instance, a graphics application that reads a camera image and find a pattern and returns a coordinate of where found. By isolating the functionality from the graphical interface, the possibility is left open that you might put the processing on a separate processor from the camera; thus, you can simply send the data needed to the processor with no graphics library at all. Also, it is easier to automate functional testing--a simple test script that executes the functions can be run separate from the graphical interface.
If you do not write it this way, it quickly becomes apparent that the task of separating the two later becomes extremely painful. Almost to the point of a rewrite.
As an aside, in looking for an A* routine, I looked at many different implementation, the "worst" ones in my opinion were the ones in which the A* routine was completely integrated into the application code and interface. It would have taken weeks to untangle the A* code from the application specific code without breaking it. The candidate ones were usually one that were designed to demonstrate the A* algorithm. Usually, you will not take the effort to seperate it because the initial effort is far greater than just embedding the application code; after all, who expects to reuse the A* algorithm in another place in the application!
Similarly, an early bad design was to incorporate the FLTK components into the lower hardware classes. This is the opposite of this separation and proved more difficult to maintain. At least, the lower stable hardware functionality code tended to get updated much more than if it were designed completely seperately. Only the GUI files or classes would get updated leaving less possibility of breaking something in the lower code. Some of this is possible, but non static objects need to have a seperate FLTK widget per object. A possible solution might be similar to the JAUS methods--to have a base class bPWM. To this we subclass to add the FLTK widgets it needs. Thus oPWM (bPWM) is not touched at all--FLTK subclasses bPWM to make oPWM whereas the 68332 code simply typedefs like the following:
#ifdef DOS
class oPWM : public bPWM
{
public:
oPWM(void);
private:
WIDGET* widget;
};
#else
typedef bPWM oPWM;
#endif
The benefit of typedefing if not DOS is it does not add extra class overhead into the embedded portion (at least shouldn't).
I think there is beginning to be a realization of the dangers of static objects--particularly in using static constructors. Since an object is static, it does not even need to be declared to be used--furthermore, it may also be declared more than once. The more than once problem can be resolved through a test for a static local init variable but the problem of construction order remains; i.e., there is no guarantee when the code in the constructor will be executed or if it even will be executed; furthermore and more serious is behavior is dependent upon the order that the objects are declared. Simply declaring a static somewhere else suddenly can introduce unforeseen bugs in the code. Rather the following is suggested:
In order to accomplish this, many of the routines may be deattached from their classes and converted to simple C style API calls with the oMobileRobot as a parameter.
Early design declared a oConsole in the main routine to open the serial port on the PC. The problem is the library code does not have access to this object. Rather than make the object static (See Previous Lesson), it is best to have each function responsible to open and close the port it uses. Thus a different port can be selected between downloads etc.
Not really a blooper, but be sure not to make something class data that can be static to a method. Many times we just have a big pot of variables in the class, when really we need a little more finese on what really needs to be class data and what can be local data at the method level and get a greater level of information hiding. This is especially true when classes tend to get larger and class data just becomes another regression to the unstructured age of programming disguised as "object oriented programming".
So stupid and obvious, yet has caught me many times, and when it does, it usually takes a LONG time to catch the fact that toggling an LED causes the program to halt! Of all the complex code, who would start there--it must be a complex timing issue, right?
OOMRM_ENTER_CRITICAL(); return value; OOMRM_EXIT_CRITICAL();
This is usually introduced "after the fact", you go back and throw in the criticals in after the code is written as no one would be so idiotic on the first writing to do it hat way, now would they?
Again, not blooper, but size_t, and wchar_t is defined in <stddef.h>. This info can be hard to find.