Re: [Discuss-gnuradio] digital_correlate_access_code_bb

[Tom Rondeau]

On Mon, Apr 9, 2012 at 2:02 PM, Nick Foster <address@hidden> wrote:
> On Mon, Apr 9, 2012 at 10:48 AM, Marcus D. Leech <address@hidden> wrote:
>>
>> On 04/09/2012 01:38 PM, Tom Rondeau wrote:
>>>
>>> On Sat, Apr 7, 2012 at 10:12 PM, Marcus D. Leech<address@hidden>
>>>  wrote:
>>>>
>>>> Just looking at this function:
>>>>
>>>> correlate_access_code_bb
>>>>
>>>> In the method set_access_code, it takes a string.  Which should be ASCII
>>>> '1'
>>>> and '0' characters to represent the binary sequence being
>>>>  correlated against.
>>>>
>>>> Here's a little beauty of a code snippet:
>>>>
>>>>  d_access_code = 0;
>>>>  for (unsigned i=0; i<  64; i++){
>>>>    d_access_code<<= 1;
>>>>    if (i<  len)
>>>>      d_access_code |= access_code[i]&  1;    // look at LSB only
>>>>  }
>>>>
>>>> This relies on the fact that ASCII '1' and '0' happen to have low-order
>>>> bits
>>>> of the right "flavour".  This is insanely dirty and gross and I can't
>>>>  believe we let this nonsense in the code base.
>>>>
>>>> There's no reason not to do the right thing here.
>>>>
>>>>
>>>> --
>>>> Marcus Leech
>>>> Principal Investigator
>>>> Shirleys Bay Radio Astronomy Consortium
>>>> http://www.sbrac.org
>>>
>>>
>>> Want to submit a patch?
>>>
>>> Tom
>>>
>>>
>> Attached.
>
>
> While you're patching correlate_access_code_bb, please patch
> correlate_access_code_tag_bb with the attached patch.
>
> --n

So my guess is that the use of the binary & operator is to avoid the
need for an if/if else/else branching check. It was most likely done
for efficiency. So while this patch might be the "right" way to do it
from a code perspective, it could result in slower code (on certain
machines that don't handle branch prediction well). It does make
assumptions about the correctness of the access code, though.

Tom