[chirp_devel] chirp_devel Digest, Vol 88, Issue 11

[Klaus Ruebsam]

Hi Dan,

thanks for your very detailled feedback (I really do appriciate that). 
Hopefully I did it all right except for two items:


>> +def _r2_enter_programming_mode(radio):
>> +    serial = radio.pipe
>> +
>> +    magic = "TYOGRAM"
>> +    exito = False
>> +    serial.write(CMD_STX)
>> +    for i in range(0, 5):
>> +        for j in range(0, len(magic)):
>> +            serial.write(magic[j])
>> +        ack = serial.read(1)
>> +
>> +        try:
>> +            if ack == CMD_ACK:
>> +                exito = True
>> +                break
>> +        except:
>> +            LOG.debug("Attempt #%s, failed, trying again" % i)
>> +            pass
> I don't think there's anything in this try..except block that can 
> raise, so I would just remove it.

No that should stay in there as we do expect to receive an ACK from the 
radio upon sending TYGRAM to the radio.
>> +
>> +    # check if we had EXITO
>> +    if exito is False:
>> +        msg = "The radio did not accept program mode after five 
>> tries.\n"
>> +        msg += "Check you interface cable and power cycle your radio."
>> +        raise errors.RadioError(msg)
>> +
>> +    try:
>> +        serial.write(CMD_STX)
>> +        ident = serial.read(8)
>> +    except:
>> +        _r2_exit_programming_mode(radio)
>> +        raise errors.RadioError("Error communicating with radio")
>> +
>> +    # No idea yet what the next 7 bytes stand for
>> +    # as long as they start with ACK we are fine
>> +    if not ident.startswith(CMD_ACK):
>> +        _r2_exit_programming_mode(radio)
>> +        LOG.debug(util.hexprint(ident))
>> +        raise errors.RadioError("Radio returned unknown 
>> identification string")
>> +
>> +    try:
>> +        serial.write(CMD_ACK)
>> +        ack = serial.read(1)
>> +    except:
>> +        _r2_exit_programming_mode(radio)
> It would probably be best to wrap this function in something that 
> always tries to exit programming mode if it raises so you don't need 
> to have it on all the exit paths. Something like
>
>   def _r2_enter_programming_mode(radio):
>       try:
>           return _really_enter(radio)
>       except:
>           _r2_exit_programming_mode(radio)
>           raise  # will re-raise the original error

I will bare that in mind but due to the fact that I am really an 
absolute beginner to Python I would like to postpone the requested 
update for later. Hopefully that is OK for you. Promissed: I will later 
on try to get that done as suggested!

>> diff --git a/tests/images/Radioddity_R2.img 
>> b/tests/images/Radioddity_R2.img
>> new file mode 100644
>> index 
>> e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..0bbd8468beda1c4f90b1be24385a530da662c710
>> GIT binary patch
>> literal 1181
>> zc%1Ez&1!={6h>{KK0}w?WS`nva2dQPBZPt>@n<!o1T$({X*&e7>$=OlXCI;V-kDit
>> z({&-2bHM92&WGwGK%o<kLT4 at qQt#@PBS7kXcFz$YO&)RtNRw|k0;I`z90AhgpBw?w
>> z<R^{*X>wN}K-zwR0BQRL0;KI12#~g4AVAuFfdFay1p=h)7wYW)FLRmyG+#H4>p0W3
>> zUVi>QpL%zCoT=W)x8Ymv8|P{}CSIBOUG>1^vs}paKI%jkbY<Mnw=4F}!sMg!(!L7W
>> z6F#q&Qon0O+>dn>cx9d(@w{T~Ys1=wT8 at v0GTwMh9vb$Nv{f7(>(P8$Wa)r+bs|sw
>> Vb!+!g-{%#1G8CCV*Z0UI_yseAoGAbR
> This looks like maybe you committed the image in a previous changeset 
> and then just tweaked it in this one? Either add it in the same patch, 
> or you can attach it to the issue you create and I can snag it from 
> there.

I binary attached it to issue #6063

> Also, I know you said you were having trouble running the style checks 
> on windows, so I have included the output of it for you at the end here.

Thanks. That helped quite a lot.

> Otherwise this looks really close, thanks! I'd like to get the image 
> so I can run the tests and get as much of the style and other nits 
> above cleaned up and then I'm good with it.

If I did it all right, the updated/patched  version should already be 
received by your system.

Kind regards,
Klaus
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20180829/f78683a9/attachment-0001.html