RDS: fix flags decoding #1361
RDS: fix flags decoding #1361
Conversation
|
Thanks for the fix, and sorry for the delay in reviewing it.
The mono/stereo issue was fixed in gr-rds here: bastibl/gr-rds#66, but it looks like the |
src/dsp/rds/parser_impl.cc
Outdated
| @@ -103,16 +103,16 @@ void parser_impl::decode_type0(unsigned int *group, bool B) { | |||
| /* see page 41, table 9 of the standard */ | |||
| switch (segment_address) { | |||
| case 0: | |||
| mono_stereo=decoder_control_bit; | |||
| static_pty=!decoder_control_bit; | |||
There was a problem hiding this comment.
I think it may make sense to leave this bit as is, and change the display (in dockrds.cpp) instead. That way there won't be an API change in the PMT message format, which could potentially break applications that depend on gr-rds.
| static_pty=!decoder_control_bit; | |
| static_pty=decoder_control_bit; |
There was a problem hiding this comment.
Hmmm.
I've tried to preserve compatibility by keeping PMT message characters meaning unchanged.
This bit means "dynamic PTY" when set, but the corresponding PMT message character means "static PTY". That's why it is inverted here.
There was a problem hiding this comment.
the corresponding PMT message character means "static PTY"
I suppose that may have been the intention, since the variable is named static_pty. But it could also be that the choice of variable name was a mistake, and it should have been named dynamic_pty.
I think the less confusing choice would be to copy the DI bits into the PMT message verbatim (as they are currently) and rename the variable.
In any case, it's gr-rds's API so we should probably see what @bastibl would prefer to do upstream.
Change d0...d3 order to d3...d0 to match the standard. Change decoding of Mono/Stereo flag to match the standard. Change decoding of stPTY flag to match the standard.
vladisslav2011
force-pushed
the
rds_flags_fix
branch
from
7d8fbc5
to
2460c4c
Compare
|
The changes look good. I have proposed the fix upstream in bastibl/gr-rds#78 and will give @bastibl a bit of time to look it over before merging this. |
|
It's been a while and I haven't heard back on the upstream PR, so I'll merge this and we can re-synchronize with gr-rds if needed. |
|
The upstream PR was accepted. Thanks for having a look @bastibl! |
Change d0...d3 order to d3...d0 to match the standard.
Change decoding of Mono/Stereo flag to match the standard.
Change decoding of stPTY flag to match the standard.
Fixes #1360