OBD MPGuino gauge

[Magister]

well, the protocol is easy to find on the net anyway (how the header/checksum work, etc), and for the PIDs almost all of them are on wikipedia
OBD-II PIDs - Wikipedia, the free encyclopedia

[ishiyakazuo]

Originally Posted by dcb

You will be putting me in a very difficult position if you do.

That's fine. I don't think it's necessary to do, anyway -- these protocols are pretty straightforward. I think if we use ELM327 in the initial design, and then work on porting over protocols from open source implementations, we should be safe both from a legal/moral standpoint, as well as in mitigating risk.

I'm not trying to suggest anything either legally or morally wrong here. I do thank you for letting me know ahead of time if you think my suggestions breach standards. As they say, ignorance of the law isn't an excuse

In any case, it sounds like we have a path forward now. My opinion is that we should pursue all three options, but attempt to keep a common code base for the Arduino projects (w/ and w/o the ELM). I'm seeing this as a three-phase project:

1) Get the ELM327 working with Arduino
2) Get ISO/VPW working on Arduino
3) Port the ISO/VPW code to AT90CAN and add PWM and CAN.

This method provides people with the most choice over time. It guarantees that we have an Arduino implementation for ISO/VPW that people with Arduinos can pull down, and it gives us a good head-start on something to test the AT90CAN out with.

Thoughts?

[dcb]

Well, you and magister have a 327 en-route presumably. I've got a 323 so I can kind of play along. I think the 327 is the shortest path and nominate Magister to lead that effort, partly because he has a CAN car and I want him to have a working gauge ASAP But he has already demonstrated initiative in that.

So my vote is obduino 1.0 is arduino + elm327 (with code compatibility for other elm variants).

You seem to have a handle on ISO so you could get started on that on an arduino for now, till we decide to move off arduino anyway. I've still got some mpguino work to do. But if Magister can use your help then he can co-ordinate that.

k?

[ishiyakazuo]

I think that route sounds best -- we all have personal interest in each of our tasks this way. I still need to get the Freescale part for the ISO bus, but I think most of the other parts I need for a crude prototype are on the way. I'll put that together and take a quick stab at ISO, and if I run into problems I'll throw an ELM327 on order immediately and work with Magister to get something working quickly if he hasn't got everything sorted by then.

We should keep the code synched up between the ISO/VPM and the ELM one. We can differentiate using some #define or similar to decide which code is being run (it makes it less confusing to deal with a single code base). We might need to do a bit of restructuring of how the messages are sent, to make some kind of generic messaging interface. I'll toss out some suggestions over the next day or two.

[Magister]

I agree with that, locally I already took the code and put some "#ifdef ELM" here and there, in fact around the iso_*() functions, and I use the Arduino IDE built-in serial port monitoring to send by hand the answer like "41 01 nn nn" in response to the pid asked by the duino board, then on the LCD I can "see" the result and all.
I'll order an ELM327 asap.

[ishiyakazuo]

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.

[dcb]

LOL, I would recommend starting with getting a distance and a fuel consumption and MPG displayed on the LCD There's plenty of real problems to sort out (like what pids are supported and which ones to use and are they vehicle specific) You can refactor as neccesary later on.

[ishiyakazuo]

You have a point, but I've been bitten too many times by not thinking up front about designing a basic API first. Design helps things get done faster and easier -- the problem is that once something is working, refactoring NEVER gets done by the person/people who made the original code. In my experience, it tends to cause a situation where new people who inherit the code don't want to change things either because they think might have some hidden magic, or because they don't want to cause offense to the original authors. So usually you wind up with a lot of hacks stitched on because the original code wasn't planned out for future enhancements.

Put simply, one of the worst things you can do in code is not break things out into pieces. And another is breaking things down too far. (Funny how it works that way.)
If you don't break things out enough, you wind up with a lot of code that's difficult to read (functions that are too long that really aren't performing a specific function, etc.).
If you break things down too much, you'll probably find yourself copying and pasting little pieces of code all over the place.

But anyway, you're right -- we need to get MPG showing. My understanding is that there's two basic methods, and if those fall through, you're in trouble. For cases such as that, I'd recommend having a function like GetMPG( lcdLocation ) which just knows how to get MPG using the current setup and puts it on the LCD somewhere nice.

[dcb]

Take a look at the mpguino/obduino code, there are functions for getting numbers and formatting them and displaying them and organizing the data by trip objects and utilizing the LCD and buttons. It is where Magister has already started. You don't need to reinvent all that as far as I know. You just need to replace the fuel injection timing bits with your obd functions (i.e. determine the elapsed fuel and mileage and add them to the trip accumulators every cycle), if we could #ifdef elm vs injector monitor at some point that might be good.

Basically each trip instance has it's own mpg, mph, gallons() functions that return a 32 bit number representing the value in 1000ths. It utilizes custom 64 bit math function to keep things simple that it can promote the values and multiply/divide them without being overly concerned with overflow/underflow, and it has a format function to convert the values (in 1000ths) to a rounded 2 decimal point string. Plus LCD positioning and displaying functions. Plus a lot I have probably failed to mention:

Revision 63: /trunk

[ishiyakazuo]

Yeah, I know roughly what's there from when I looked at it the other evening -- sorry if I'm saying obvious things, since I haven't really looked at it for a little while.

In any case, I did notice a handful of direct calls to iso_write...() and iso_read...() that I thought should probably be abstracted to just a "get me this data" call. That stood out the most. Most of the code should know absolutely nothing about ISO, VPW, CAN, or even using the VSS line itself.

(From what I saw, there was no real attempt to keep the MPGuino code working in this code, which I think is a bit of a shame, since it could just be abstracted out as another bus. This is the main reason I'm on my soapbox )