[chirp_devel] [PATCH]

[KS]

> Thanks a lot for working on this, I know some of the comments below are likely inherited from existing code, but I’d appreciate it if you could address them. I can also help make some of the changes if need be.
>
>> +# SCRAMBLER = ["Off"] + ["0" + str(x) for x in range(1, 8)]
>> +# memory slot 0 is not used, start at 1 (so need 1000 slots, not 999)
>> +# structure elements whose name starts with x are currently unidentified
>> +_MEM_FORMAT = “""
> Should be a blank line before _MEM_FORMAT.
>
>> +    def _write_record(self, cmd, payload=None):
>> +        # build the packet
>> +        _header = '\x7a' + chr(cmd) + '\xff'
>> +
>> +        _length = 0
>> +        if payload:
>> +            _length = len(payload)
>> +
>> +        # update the length field
>> +        _header += chr(_length)
>> +        if payload:
>> +            # calculate checksum then add it with the payload to the packet and encrypt
>> +            crc = self._checksum(_header[1:]+payload)
> There are a lot of places in this file where an operator (+ in this case) needs to have whitespace around it. I’m not sure why our style checker isn’t complaining about that — I imagine it’s because we’re not fully clean in all our files. It’d be nice if we could try to be good about adding new stuff in proper format.
>
>> +    def decrypt(self, data):
>> +        result = ''
>> +        for i in range(len(data)-1,0,-1):
>> +            result+=self.strxor(data[i],data[i-1])
>> +        result+=self.strxor(data[0],'\x57')
>> +        return result[::-1]
> I remember people claiming that this radio “encrypted” the conversation and I balked at the idea. Definitely looks like some intentional obscuring going on here though. Kudos to whoever figured that out.
>
>> +
>> +    def encrypt(self, data):
>> +        result=self.strxor('\x57',data[0])
>> +        for i in range(1,len(data),1):
>> +            result+=self.strxor(result[i-1],data[i])
>> +        return result
>> +
>> +    def strxor (self, xora, xorb):
>> +        return chr(ord(xora) ^ ord(xorb))
>> +# Identify the radio
> Needs another blank here.
>> +    def _do_download(self, start, end, blocksize):
>> +        # allocate & fill memory
>> +        image = ""
>> +        for i in range(start, end, blocksize):
>> +            #req = chr(95) + chr(i % 256) + chr(blocksize)
> Please remove commented code, especially if it’s not right. It’s just confusing later.
>
>> +                                                              smuteset]))
>> +        cfg_grp.append(rs)
>> +        #_pwd = "".join(map(chr, _settings.mode_sw_pwd))
>> +        #val = RadioSettingValueString(0, 6, _pwd)
>> +        #val.set_mutable(True)
>> +        #rs = RadioSetting("mode_sw_pwd", "Mode Switch Password", val)
>> +        #cfg_grp.append(rs)
>> +        #_pwd = "".join(map(chr, _settings.reset_pwd))
>> +        #val = RadioSettingValueString(0, 6, _pwd)
>> +        #val.set_mutable(True)
>> +        #rs = RadioSetting("reset_pwd", "Reset Password", val)
>> +        #cfg_grp.append(rs)
> Here too.
>
>> +        #
>> +        # Scan Group Settings
>> +        #
>> +        # settings:
>> +        #   u8    scg_a;
>> +        #   u8    scg_b;
>> +        #
>> +        #   struct {
>> +        #       u16    lower;
>> +        #       u16    upper;
>> +        #   } scan_groups[10];
> Why is this here?
>
> Thanks!
>
> —Dan

Appreciate for that, Dan. I forgot to remove commented/unused lines and 
ofc I will fix the styling.
Obscuring the transmission wasn't a big challenge :) Thanks to the basic 
binary operations I could find the algorithm they did use.
Why they did this - who knows, definately that wasn't good idea.
I belive they using the same for higher models like KG-UV9D but it's 
need to be checked.

Kind regards,
Krystian
_______________________________________________
chirp_devel mailing list
chirp_devel at intrepid.danplanet.com
http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers