Simple Serial Cleanup V00J

Because of the size and complexity of simple_serial.c, I decided I would work on cleaning it up for this week’s post.

To start with, I created two new files for postable events called events.c and events.h.  I moved the function task_handler() from simple_serial.c to events.c and added a //TODO comment to remind me to clean up the task handler code.  This created a bunch of errors because I haven’t included any of the library header files.  So I stepped through the process of adding the necessary header files.  I copied all of the include statements from simple serial.c to events.c and eclipse reported no errors.  Next, I commented out one line at a time of the include statements. When eclipse indicated errors in the file, I uncommented that line and moved on to the next.  I ended up only needing two include statements: uart.h and user_interface.h.   I moved the prototype of task_handler from simple_serial.h to events.h. Then I put an include statement into simple_serial.c for events.h.

I decided this was a good point to build to see if I had broken anything. It failed to build.  The error I got was

In file included from simple_serial.c:22:0:
../include/events.h:14:1: error: 'task_handler' used but never defined [-Werror]
 task_handler(os_event_t *);
 ^

The LOCAL keyword was making task handler only available to events.c.  I removed the LOCAL keyword from both events.h and events.c and got a new error.

user/.output/eagle/debug/lib/libuser.a(events.o):(.irom0.text+0x2c): undefined reference to `os_sprintf'

It looks like eclipse did not report an error when I removed one of the include lines.  I went back and added one of them at at time till the error went away.  The missing include was osapi.h  The code built and I tested it on the board. It booted and connected to the wifi as expected.

Next I created some macros that I will use when posting a task for serial events.  Now serial_init() is much easier to understand.

Next, I took on simple_config_ui().  First I started by looking at the biggest case statement which had another switch/case statement enclosed. When menu_stat is equal to IDLE, there is a switch/case for handling received characters;  I created a function called recv_byte(char recvd), copied the code and made a few little changes to make it compile and run.  I continued this process of taking large chunks of code and putting them into functions.

Finally I created a new uart.c and moved the code I originally got from uart.c out of simple_serial.c into uart.c. It took a little fiddling to get it all working again.  I have uploaded this update to Github.  Click the firmware link near the top of the right hand column to download it.

Are you cleaning up code? Did you learn to write “easy to read” code in school? How did you learn to write code? Are you learning to write code now?