Add support for Mode 9 PIDS

[alistair23]

This is baed on the original work by Paul Mundt (https://github.com/brendan-w/python-OBD/pull/151).

This patch includes adding a simple git test case.

This will finally fix: https://github.com/brendan-w/python-OBD/issues/42

Signed-off-by: Alistair Francis alistair@alistair23.me

[alistair23]

@Ircama @BedoM @pmundt

[Ircama]

@alistair23 ok will do in the next days

[mabed-fr]

Yes I can test on my car 🚗

[alistair23]

Has anyone had a chance to test this?

[zeh-almeida]

Has anyone had a chance to test this?

@alistair23 I copied the code to my project like this:

LOGGER = obd_utils.get_logger(__name__)

def _encoded_string(length):
    """ Extract an encoded string from multi-part messages """
    return functools.partial(_decode_encoded_string, length=length)

def _decode_encoded_string(messages, length):
    """ Decodes a binary message with a minimum length from OBD response as a string """
    data = messages[0].data[2:]

    # If a bogus string length is returned, discard it
    if len(data) < length:
        LOGGER.debug('Invalid string returned. Discarding: %s', data)
        return ''

    LOGGER.debug('decoded message: %s', data)

    # Encoded strings come in bundles of messages with leading null values to
    # pad out the string to the next full message size. We strip off the
    # leading null characters here and return the resulting string.
    data = data.strip().strip(b'\x00').strip(b'\x01').strip(b'\x02').strip(b'\\x00').strip(b'\\x01').strip(b'\\x02')

    LOGGER.debug('stripped decoded message: %d', data)
    return data.decode()

# Command for querying the Vehicle Identification Number from OBD
VIN = OBDCommand('VIN',  # name
                 'Vehicle Identification Number',  # description
                 b"0902",  # command
                 22,  # number of bytes to expect in return
                 _encoded_string(17),  # decoding function
                 ECU.ENGINE,  # (optional) ECU filter
                 True)  # (optional) allow a "01" to be added for speed

As you can see, I added the

data = data.strip().strip(b'\x00').strip(b'\x01').strip(b'\x02').strip(b'\\x00').strip(b'\\x01').strip(b'\\x02')
data.decode()

When testing, my response looked like: bytearray(b'\\x0<VALUE>\\x00\\x00').

It is important to note that I could only run the command with the regular connection. Maybe I did something wrong when declaring my code because if I try to run using the async connection, it returns null.

I got it to work with async connections, just have to wait a little for the callback to be read first.

[alistair23]

Thanks for testing that @zehpavora.

I don't like that you have to strip() the response so much, especially as my patch already has a few calls to the strip() function. I think we need a better solution for that.

[zeh-almeida]

@alistair23 is there anything I could do to help you have those things mainlined?

[alistair23]

I just pushed an update, can you try that and see if that removes the requirement on all the strip() calls?

[zeh-almeida]

I just pushed an update, can you try that and see if that removes the requirement on all the strip() calls?

I couldn't test it directly on the car but by using the Python CLI, the d variable will not be changed in place by the strip() calls, you have to reassign it at obd_decoders.py@509: return d.strip().strip(b'\x00' b'\x01' b'\x02' b'\\x00' b'\\x01' b'\\x02')

[alistair23]

Updated!

[zeh-almeida]

Updated!

~~Sorry for the late reply! I've updated my project using this branch and once I test it, I'll edit this comment.~~

@alistair23: Alright, I got to test this PR with my car. While not all commands had a response, I got at least some.

If it helps, I used a Ford Fiesta SEL 2018 from Brazil.

OBD_COMPLIANCE: 'Brazil OBD Phase 2 (OBDBr-2)'
PIDS_9A: '0101010001000000000000000000000000000000'
VIN: '<OK but I redacted it>' 
CALIBRATION_ID: '<OK but I redacted it>' 
CVN: '<OK but I redacted it>' 

Please let me know how I can help.

[alistair23]

Thanks for testing! I tested the latest version on my side as well and it seems to work. I'm going to merge this PR

[austinlg96]

I think that there is a flaw in line 508 of decoders.py: d.strip().strip(b'\x00' b'\x01' b'\x02' b'\\x00' b'\\x01' b'\\x02')

My vehicle's VIN starts with '1' and ends with '2' and both characters get removed whenever line 508 is cleaning up the VIN.

My VIN is essentially 1XXXXXXXXXXXXXXX2 d in that function is thus bytearray(b'\x011XXXXXXXXXXXXXXX2\x00\x00')

bytearray(b'\x011XXXXXXXXXXXXXXX2\x00\x00').strip().strip(b'\x00' b'\x01' b'\x02' b'\\x00' b'\\x01' b'\\x02') is bytearray(b'XXXXXXXXXXXXXXXX')

I'd be interested in submitting a PR to fix this. Chopping off b'\\x01' and b'\\x02' fixes it in my case, but I'm having some trouble understanding why they are there in the first place to make sure it wouldn't just cause more issues.

Does anyone recall why they are necessary?

If my interpretation of the code is right, .strip(b'\\x00' b'\\x01' b'\\x02') is essentially no different from .strip(b'\\' b'x' b'0' b'1' b'2') or .strip(b'\\x012') (that is to say it is removing \x5C, \x78, \x30, \x31, \x32, and not \x00, \x01, or \x02). Are b'\\x00', b'\\x01', and b'\\x02' all maybe unnecessary?

[alistair23]

From memory I was having trouble stripping extra characters, which is why this is there