PCM Hammer data filtering improvement help needed

[antus]

Hi,

Question to those with more C# skills than myself. I am trying to solve this annoyance where PCM Hammer gets out of sync at the end of a read on some PCMs and requires a device reset before starting another operation. I could easily put in a hack to flush the buffer at the start of a new operation, but I'd rather improve data filtering, which would probably also improve reliability when flashing in a car.

Here is an example. At the end of the previous flash process. This example is on the bench, a vehicle bus would be more noisy. PCM Hammer finishes the flash process and sends out a couple more tool present broadcasts (8C FE F0 14 meaning 8C Priority (not important) FE=Target is broadcast, F0=from Tool, 14=Tool Present) before everything wraps up. Even though they are sent to a broadcast address FE, the PCM responds to the tool with 8C F0 10 54 meaning from PCM 10, to Tool F0, Mode 54 response to mode 14. Essentially an Ack that does nothing.

The problem is, it genuinely is a packet from the PCM to the Tool, just not one we care about. So then when the next operation begins, PCM Hammer sends a mode 3C packet for data 01, and even though we see the PCM responds correctly with a mode 7C 01 in the universal patcher logger, PCM Hammer has nothing in it to ignore or filter the previous 2 tool present responses, and breaks as it is out of sync.

So the thought is, that we need to implement filtering better. Device ID isnt going to help here, rather I think something in the device or protocol layer needs to know that it sent a mode 3C, then it should receive and discard for a second of two and search for the expected response eg 7C, then pass that back. Then the top level logic can just send its 3C, receive its 7C and the other packets wont get in the way.

The hard part is, how to do this cleanly. Ive had a look, I could hack something in to receive in device.cs to just discard unexpected packets by prioritiy which would hide this and make this particular scenario work, but its an abuse of the priority and not a clean solution.

There is similar verification code here https://github.com/LegacyNsfw/PcmHacks/ ... Utility.cs
But none of that searches for the received messages. The logic probably should go in the protocol class somewhere, though. Then some of the flow logic could be moved out the places it is repeated, and this code could be used more widely improving pcmhammer overall.

I don't do C# or OO programming as a day job and I don't have a strong network of C# devs around me to suggest the best place or best path forward. Ideas to solve this properly welcome.

Would a state machine that sets the next expected packet globally help? Does it belong in the API to protocol.cs in each call and the app updated to that even though the best device drivers stream data the main logic can send a packet and get back a matched response? Other ideas?

[Tazzi]

In dealership software (and most J2534 apps), they add delays between events where the data is not cared about, and simply 'clear buffers' to ignore everything received during that period.

It sounds like a hack, but it is done intentionally since it is typically a point of clearing faults, then returning a module back to a normal state and then clearing faults again.
To which it doesnt 'really' care about getting a positive or negative response as it is not critical for it to complete the flashing sequence.

This is also done as dealership software is setup to listen to multiple devices at once. You cannot hard code the number of devices it will answer back from since it can vary between vehicles, along with there may be missing modules or faulty modules which do not respond.
Hence, the most simple and effective, is just a small pause (1500ms) and then clear buffers

[antus]

Thanks for the input, but I disagree with that. While I am not a C# OO dev I have written high performance C code in a commercial setting and I know relying on timing and nothing else happening when you don't expect it is not a good way to handle communication networks. Its probably why the factory software has so many problems. Even though PCM Hammer is free software, its going to be better than that (hopefully)

I'll use this example as one way of doing it. This is from Dimented24x7's Black Box programmer. Source was shared with me by Dimented before he left the scene. I do not have permission to re-share in full, but this snippet will describe the state machine option.

Code: Select all

        //Declare global value to search incoming OBD data stream
        //Values: 
        //"ID_AVT" = Identify cable hooked to PC
        //"1X_OK" = Waiting to start 1x mode with AVT
        //"ECHO" = Echo data to console window for diag. aid
        //"SECURITY_SEED" = Waiting to recieve security seed from PCM
        //"SECURITY_OK" = Waiting for PCM to grant security access
        //"4X_OK" = Waiting for PCM to grant 4X mode 
        //"MODE34_OK" = Waiting for PCM to allow mode 34 access, used to upload kernal
        //"CALL_PCM" = Establish kernal is running and PCM ready
        //"PCM_READY" = PCM is ready to accept uploads/process downloads
        //"START_DOWNLOAD" = Request first frame of download for bin from flash
        //"DOWNLOAD" = Download bin from flash
        //"UPLOAD" = Upload bin to flash

Code: Select all

        public void startAVT()
        {
            bool commanderror = false;
            string[] restartavt = { "F1", "A5" };
            string[] idavt = { "F0" };
            byte[] sendcommand;
            //Restart AVT
            sendcommand = convertforserialarray(restartavt, out commanderror);
            WriteSerialData(sendcommand, true);
            Thread.Sleep(200); //Let the AVT restart
            //Request ID of AVT
            sendcommand = convertforserialarray(idavt, out commanderror);
            WriteSerialData(sendcommand, true);
            searchforframe = "ID_AVT";
        }

Here he uses a state machine to set the global comms reader know what to do. It has the delay, although that could be considered optional here, and it'll read and discard packets until it finds the matching one for the ID_AVT state. Then elsewhere it knows what to do from there, and call the next function in the flow.

My experience with state machines was for SS7 telephone networks (although the electrical protocol was ETS300 in Australia). We had several thousand channels of phone calls in parallel on each machine and you could get any network switching message at any time on any channel. Calls from different companies or countries or types of networks all acted slightly different and it all had to run fast because people expected instant call switching, and stable... obviously. There was a catch all to log an unknown state if something happened we had never seen before, and we could easily map the logic to do something else, and write another function if it needed special treatment. We used a similar to the above state machine and a main logic loop that looked at the current state and a decision tree what to do when different messages came in at different times.

The challenge is, we havn't written pcmhammer with a state machine. The question is, is it worth doing such a heavy change? Or do we just put the wait logic in the API between the main control flow and the interface / network, a bit of a half way solution.

Whatever we do, I think we need it to discard unexpected packets. So not only does it fix this issue, it can handle it when a busted module gets vocal and it should not, or a stereo breaks the standard.

[Tazzi]

Totally agree with being better then dealership software.

But there is also no need for a gloabl state machine, since there doesn't need to be a global monitoring for it.
Unless PCMHammer was to move to a multi threaded setup where several routines are running co-currently and all needing to communicate with a scantool plus network at the same time, otherwise it is never needed.

The exception to this would be if logging and flashing where 'combined' where a logging thread could seamlessly integrate with a flashing thread for something like realtime flashing and logging.. although thats a bit off topic.

The way I do it (And pcmhammer has it for the mode 28 disable chatter command), is to:
1) Send the required command
2) Search for any message for X amount of time in the internal frame search routine (Waiting for the message to actually come from the scantool).
3) If frame is found AND matches what you are after, restart a timer to look for next message
4) If frame is found and does not match what you are after, keep timer rolling, exit while loop once the overall timeout is reached.

This allows searching for all valid frames and allows the system to complete search for all 'applicable' messages within an allocated time out.

The above setup is also only required when doing a vehicle wide request where multiple modules will respond back.

Outside of that, the software should only be looking at responses from the actual module it wants to be communicating to.
When a frame is not a global message, we only should look at the module response, so the header ID should be used to filter out any undesired messages regardless of when they are received.

But, if a message is received from the desired module which is not as expected, for example, 6C F0 10 60, then that message should be taken into consideration since if that occurred before requesting to upload, or some other point, then it means the ecu has returned to normal state which does alter the flow of actions that should take place next (Instead of ignoring and discarding).


As for delays, they are really important to be using (But in the right way).
The biggest offender being high CPU usage and locking up a serial port from updating its internal concurrent queue if constantly polling the "available" number of bytes on a port.

PCMHammer has a similar setup here, but the easiest of design for waiting for all messages to be received while handling bad messages
ie...

Code: Select all

While (timer.elapsedmilliseconds < MaxIdleTimeout)
 {
	var NewData = await ReadFrame(FilterID);
	if (NewData == MyWantedResponse)
	{
		//Perform logic for RXD message
		timer.restart();		
	}
	else //Check message is not 7F or other important message, else pause momentarily
	{
	       //code logic to handle error messages or unwanted messages
		await Task.Delay(50);
	}
}

[kur4o]

Mode FE 14 is clearing DTC from all modules.

I see you have issues with processing FE responses. In custom communication priority should be ignored. It is targeted for use on normal bus communication.

Responses to a random FE mode should be handled as to a response per X count of modules, where you listen for target to tool + 40 to mode, if positive or mirrored message with 7f as negative response.

than you set a timeout for listening to valid responses to 1500-2000ms from the last valid response, before you move to next frame.

You need transparent full communication to work, as per elm will likely not work unless you use built in filtering of elm.

[antus]

yeah. the question is more for c# devs, about where and how to put it in the app. How far its worth to go to improve comms handling.

[antus]

Thinking about it overnight I think the place is the protocol class. I can extend the transmit and receive functionality, so that call to transmit sets an internal variable in that class which receive can use to filter the incoming packets. Once a packet is received it can clear the filter. Then I can add another definition for the receive function to take an optional argument to skip the receive filtering, so that I can modify the tool present transmit etc, and that can do transmit only and leave receive filtering off. That will be the lightest touch approach and should only require minimal changes to the app. Some of the places that have filtering in the wrong spot can then have the filtering removed, the outcome will likely be better results app wide and less code in total.