[chirp_devel] [PATCH]

[Dan Smith]

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