[PATCH 02/11] This adds code to read OSTC3 firmware files.

[Jef Driesen]

On 15-12-14 23:28, Anton Lundin wrote:
> On 15 December, 2014 - Jef Driesen wrote:
>>> +static unsigned int hw_ostc3_firmware_checksum(hw_ostc3_firmware_t *firmware)
>>> +{
>>> +	unsigned short low = 0;
>>> +	unsigned short high = 0;
>>> +	for(unsigned int i = 0;i < SZ_FIRMWARE;i++)
>>> +	{
>>> +		low  += firmware->data[i];
>>> +		high += low;
>>> +	}
>>> +	return (((unsigned int)high) << 16) + low;
>>> +}
>>
>> This looks like a variant of the fletcher16 checksum (with a 16bit sum
>> instead of 8bit sum, and modulo 2^n instead of 2^n-1). So I suggest moving
>> it to checksum.c where it can be reused elsewhere.
>>
>
> Is that such a wise idea? Sure, its a cousin of fletcher16, but do you
> think anyone else will be using the same cousin of fletcher16?
>
> If it where a straight fletcher16, i would have agreed with you but this
> one, I'm not so sure. Just think, what should we name the darn thing
> when we move it to checksum.c? ostc3_firmware_fletcher16_almost()?

Well, it's not really a must for me. Having all checksums in a single file is 
just convenient when trying to reverse engineer some unknown checksum. I can 
simply try all checksums in that file, without having to remember whether there 
are any others elsewhere. That's all.

The checksum.c file already contains the custom checksum_add_uint4 that's only 
used in a single place. The CCITT is also one specific variant of the CRC algorithm.

For the name we could pick checksum_fletcher_uint32, or something like that.

But as I said, it's not really a must.

>>> +static dc_status_t
>>> +hw_ostc3_firmware_readline(FILE *fp, unsigned int addr, char data[], unsigned int size)
>>> +{
>>> +	char ascii[40];
>>> +	// 1 byte :, 6 bytes addr, X*2 bytes hex -> X bytes data.
>>> +	unsigned line_size = size * 2 + 1 + 6 + 1;
>>> +	unsigned char faddr_byte[3];
>>> +	unsigned int faddr = 0;
>>> +	if (line_size > sizeof(ascii))
>>> +		return DC_STATUS_INVALIDARGS;
>>> +	if (fread (ascii, sizeof(char), line_size, fp) != line_size)
>>> +		return DC_STATUS_IO;
>>> +	if (ascii[0] != ':')
>>> +		return DC_STATUS_IO;
>>> +	if (array_convert_hex2bin(ascii + 1, 6, faddr_byte, sizeof(faddr_byte)) != 0) {
>>> +		return DC_STATUS_IO;
>>> +	}
>>> +	faddr = array_uint24_be (faddr_byte);
>>> +	if (faddr != addr)
>>> +		return DC_STATUS_IO;
>>> +	if (array_convert_hex2bin (ascii + 1 + 6, size*2, data, size) != 0)
>>> +		return DC_STATUS_IO;
>>> +
>>> +	return DC_STATUS_SUCCESS;
>>> +}
>>
>> This will needs some improvements to be able to deal with different line
>> endings (e.g CRLF vs LF). See ihex.c for an example.
>
> None of the Ostc3 or Ostc Sport firmware files have CRLF endings.
> Probably due to the tool with which they bin generated.
>
> But, when re-reading the HexFile::loadEncrypted of ostc-planner, it
> would accept a file with CRLF encodings, so we should do the same.
>
> Fixed.

Even if the files are generated with one particular line ending, we can't really 
guarantee that the original line ending is preserved when those files are send 
as text files over email, ftp, etc. So I think it's a nice feature to be able to 
handle this transparently. Especially because it's not very difficult to do. 
After all, it might save people some frustration.

>> Two questions here:
>>
>> If I understand correctly, the address in the firmware file is not really an
>> address in the normal sense, but more like a sequence number, right? Can
>> they be in a random order, or are they always nicely incrementing as the
>> code assumes?
>>
>
> All the firmware files I've seen, they are always linear. The
> ostc-planner code also assumes that they always will be and will fail
> reading the file if they aren't, so thats the reason this code will
> reject such a file to.
>
>> Is the payload always guaranteed to be 16 bytes (except for the last line
>> with the checksum)? I assume it is because there is no length stored
>> anywhere.
>>
>
> I haven't seen any files in any other format. They are all the exact
> same size, 307256.
>
> If yo take a look at the decrypted data, its padded to be this exact
> firmware size.

I just wanted to double check that we don't start with wrong assumptions.

Jef