That's a good approach. I think we should really abstract things down even more though, like
GetCurrentData( PID_VSS );
which completely abstracts the concept of where you're getting the data from, but returns a number to you, which you can then use in your calculation.
GetCurrentData() would then call another function based on what is #defined in the code ( iso_write() followed by iso_read(), elm_write() followed by elm_read(), etc. )
At a very high level, you want the code to just give you the data in a quick and easy fashion, and you don't worry about how it got to you. It makes it much easier to abstract interfaces at a lower level (you only need #ifdef in a few places in your code, as opposed to all over).
One technique I used on the LCD rendering on a previous project (which was much more CPU-constrained than this one) was to store an image of the LCD's text in a local buffer, and then send it out all at once when it got some CPU time. That project also used a 2x16 LCD, and so the buffer was just a char[32].
Here's an example of how that worked:
There was a char[32] lcdBuffer defined, and there were 8 pieces of data displayed on the screen. So each piece of data had 4 characters. I just defined UPPER_LEFT to be 0, LOWER_RIGHT to be 7, and the others as appropriate. In this situation, where you have 4 data points and each one can have 8 chars, you'd have TOP_LEFT = 0, TOP_RIGHT = 1, BOTTOM_LEFT = 2, BOTTOM_RIGHT = 3. So every function that could write data to the LCD just asked where you wanted it.
In our situation here, you could just say
DisplayCalculatedData( INSTANT_MPG, TOP_LEFT );
DisplayCalculatedData( MILES_TRAVELLED, TOP_RIGHT );
DisplayCalculatedData( AVG_MPG, BOTTOM_LEFT );
DisplayCalculatedData( GALLONS_USED, BOTTOM_RIGHT );
UpdateLCD();
and then DisplayCalculatedData() would make the necessary GetCurrentData() calls to calculate out the value, perform the calculation, and store the result in an offset of lcdBuffer. Then the next time UpdateLCD() is called, it's a tight loop that cleanly sends the entire LCD image over to the physical LCD.
The other benefit this may have is the case where the LCD is being updated too quickly to read the data. You could program in the update frequency as another #define, and the only thing it'd need to change is the delay in UpdateLCD().
Thoughts on this? It's more of a semantic change to what's already there than anything else -- I just found it cleaner in previous projects I've worked on where I've run into similar situations.
|