Logger improvements

[pmcquay]

If you want someone to help, I'd be more than willing, and yeah not having the logging mess with the flashing would be priority there I'd imagine. I'd just need to know where to start so as to not step on anyone's toes, or duplicate work. I'm willing to do simple code cleanup and organization tasks as well, if things need moved around for that sort of work.

[rjdrew1986]

Just curious, what are the symptoms when Crank Relearn is needed?

[gmtech825]

If the value has never been learned than the ecm sets a dtc. otherwise some service procedures require you to relearn it for misfire detection (engine replacement).

[NSFW]

Yep. I want it for the misfire detection. What I thought was a rough cam turned out to be a flaky ignition harness, which I confirmed by using a temperature gun on the exhaust headers. I thought I had it fixed, but now I'm not sure.

[NSFW]

NSFW do you have a text version of the disassembly? so I can look at it and learn how the computer work might help me put 2 and 2 together

I do, sorry for the delay.

https://raw.githubusercontent.com/Legac ... itized.asm

At 14mb, it's quite large. I don't know what software will be able to handle it gracefully, but try Visual Studio Code if you don't have something already.

[NSFW]

If you want someone to help, I'd be more than willing, and yeah not having the logging mess with the flashing would be priority there I'd imagine. I'd just need to know where to start so as to not step on anyone's toes, or duplicate work. I'm willing to do simple code cleanup and organization tasks as well, if things need moved around for that sort of work.

There is a code-cleanup thing that's been on my mind for a looong time, but it's not exactly simple.

When I started writing the code, I had this idea that it might be handy to have most functions return an object containing two values: a status code and the value that the function was really supposed to provide. It's similar to how web servers return an HTTP status code and a page of HTML (or a JSON data structure, or XML, or whatever). So there are a ton of functions that return a Response<bool> or a Response<string> or something like that. This turned out to be a bad idea.

It's a lot easier to just return the bool or string, and throw an exception if something goes wrong. That way the exception only needs to be caught by the code that really wants to handle it. Whereas with the return-an-error-code approach, we have to check response.Status at every layer in the call stack, and explicitly return a response object with the same status code (and often a different default value). It just adds a bunch of extra code that provides no real value, and it creates a place for bugs to creep in, and makes it harder to find bugs.

I just pushed up a new branch named 'exceptions' that contains a new ObdException class.

This:

Code: Select all

return Response.Create(ResponseStatus.Success, foo) 

Should become this:

Code: Select all

return foo

This:

Code: Select all

return Response.Create(ResponseStatus.some-error, foo) 

Should become this:

Code: Select all

throw new ObdException(ObdExceptionReason.some-error, "a few words about what went wrong")

Things like this - checking for an error just to return a slightly different Response object - should probably just be removed, because exceptions will bubble up automatically. There might be some value in catching, logging a message, and re-throwing, but I suspect that in most cases that won't really be helpful.

Code: Select all

                    if (response.Status != ResponseStatus.Success)
                    {
                        logger.AddUserMessage("Failed to load loader from file.");
                        return new Response<Stream>(response.Status, null);
                    }

Something like this - checking for an error and returning something simple:

Code: Select all

                        response = await vehicle.LoadKernelFromFile(this.pcmInfo.LoaderFileName);
                        if (response.Status != ResponseStatus.Success)
                        {
                            logger.AddUserMessage("Failed to load loader from file.");
                            return false;
                        }

...should turn into this:

Code: Select all

                        try
                        {
                            file = await vehicle.LoadKernelFromFile(this.pcmInfo.LoaderFileName);
                        }
                        catch (Exception exception)
                        {
                            logger.AddUserMessage("Failed to load loader from file.");
                            logger.AddUserMessage(exception.Message);
                            return false;
                        }

I suspect that those patterns will cover most of the changes that need to be made, but I also suspect there will be places where it won't be obvious what should be done. But we can figure it out.