[chirp_devel] [PATCH 1 of 3] Simplify some of the cross tone tests exceptions

[IZ3GME Marco]

Hi Dan
I checked the proposed patch starting from wiki page 
http://chirp.danplanet.com/projects/chirp/wiki/DevelopersToneModes and 
with special attention to the usage matrix I written at the end.
I find that documentation looks correct and is expecially consistent in 
respect of tone and dtcs fields and flags usage.

Stated this, the actual patch 1 looks, to me, wrong on some usage cases 
(also after the corrections in patch 3).

The original code looks correct compared to the tone usage matrix, 
although it can be probably written in a more synthetic way. As it is 
it's easy to read being in positive logic and explicitly recall every 
use case.
Btw the code is "symmetric" on tone and dtcs checks which simplify the 
readings; once we change the dtcs checks we should replicate to the tone 
checks.

I'll add some more comment in reply to patch 3 but please don't think 
I'm "refusing" your changes just because the original code is "mine" ;)

73 de IZ3GME Marco


On 30/06/2012 03:22, Dan Smith wrote:
> # HG changeset patch
> # User Dan Smith <dsmith at danplanet.com>
> # Date 1341019319 25200
> # Node ID 6aff75e219ac3ce6953ac7d3d4768597a597529b
> # Parent  e17c096f7da6e236ef3bfdd165564dda49ab043c
> Simplify some of the cross tone tests exceptions
>
> I think that the existing ones were overly complicated _and_ not quite
> right. Specifically, checking for has_rx_dtcs is unnecessary and not
> appropriate, as tmode == "Cross" is really the only gating factor,
> and the txmode and rxmode values are what need to be checked.
>
> If you apply this patch, along with the next two, you'll see that
> we have some logical inconsistencies with radios that support
> *->DTCS cross modes, but weren't changed to support rx_dtcs.
>
> Marco, are these changes okay? Assuming the other drivers need to be
> fixed, of course...
>
> diff -r e17c096f7da6 -r 6aff75e219ac tests/run_tests
> --- a/tests/run_tests	Fri Jun 29 17:16:23 2012 -0700
> +++ b/tests/run_tests	Fri Jun 29 18:21:59 2012 -0700
> @@ -179,18 +179,10 @@
>                           )
>                       ):
>                   continue
> -            elif k == "dtcs" and not (
> -                        (a.tmode == "DTCS" and not rf.has_rx_dtcs) or
> -                        (a.tmode == "Cross" and tx_mode == "DTCS") or
> -                        (a.tmode == "Cross" and rx_mode == "DTCS" and not rf.has_rx_dtcs)
> -                        ):
> +            elif k == "dtcs" and ((a.tmode != "DTCS") or
> +                                  (a.tmode == "Cross" and tx_mode != "DTCS")):
>                   continue
> -            elif k == "rx_dtcs" and (not rf.has_rx_dtcs or
> -                        not (
> -                            a.tmode == "DTCS" or
> -                            (a.tmode == "Cross" and rx_mode == "DTCS")
> -                        )
> -                    ):
> +            elif k == "rx_dtcs" and (a.tmode == "Cross" and rx_mode != "DTCS"):
>                   continue
>               elif k == "offset" and not a.duplex:
>                   continue
> _______________________________________________
> chirp_devel mailing list
> chirp_devel at intrepid.danplanet.com
> http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
>