[chirp_devel] Patch for #8363

Dan Smith
Thu Nov 19 12:36:55 PST 2020


> @@ -18,17 +21,18 @@
>  import struct
>  import logging
>  import re
> -
> -LOG = logging.getLogger(__name__)
> -
>  from chirp import chirp_common, directory, memmap
>  from chirp import bitwise, errors, util
>  from chirp.settings import RadioSettingGroup, RadioSetting, \
>      RadioSettingValueBoolean, RadioSettingValueList, \
>      RadioSettingValueString, RadioSettingValueInteger, \
> -    RadioSettingValueFloat, RadioSettings,InvalidValueError
> +    RadioSettingValueFloat, RadioSettings, InvalidValueError
>  from textwrap import dedent

There is a ton of random noise in this patch, whitespace changes and moving things around that doesn't change anything functionally. If you want to do those cleanups, that's fine, but please make it a separate patch that doesn't change any functionality and only does cleanups. Otherwise it's very hard to bisect out breakages in the future.

>          # Now we read
>          d = _recv(radio, addr, BLOCK_SIZE)
> +        time.sleep(0.1)

This is fragile and error-prone, and only makes you less likely to encounter the problem, but does not fix it. You should check that the length of 'd' is correct, and either re-do the read with the remaining size, or raise an error. Generally what drivers do is have a reasonable-for-the-radio timeout value set, and if we read less than we're expecting, we raise an exception. This time.sleep() might make it get consistently lucky on your computer, but for someone else with a slower or faster machine, it won't. It also just slows things down unnecessarily in the case where we did get the right amount.

--Dan


More information about the chirp_devel mailing list