Re: [Discuss-gnuradio] digital_correlate_access_code_bb

[Tom Rondeau]

On Sun, Apr 15, 2012 at 6:30 PM, Nowlan, Sean
<address@hidden> wrote:
> Sorry to bump this... It appears the set_access_code method only occurs once  
> from within the constructor code. I'm not arguing that the old method isn't 
> faster, but it's functionally imprecise and the overhead of the if-else 
> statement isn't huge over the life of the object instance. If a time-variable 
> access code scheme were implemented, I could see it adding up, though. But 
> set_access_code isn't even SWIGged up as a public method, so I assume there 
> hasn't been demand for such a use case...
>
> Sean

No, not really much demand. But I would suggest we relook at this not
from the processing standpoint but from the definition of the access
code.

It's inputted as a string right now and then converted to a 64-bit
value. Presumably, the string definition was used to make arbitrary
length access codes possible and easily passed through SWIG. Could we
instead define the access code another way that allows us to fix both
the representation and processing issues?

Tom


> -----Original Message-----
> From: address@hidden [mailto:address@hidden On Behalf Of Tom Rondeau
> Sent: Monday, April 09, 2012 5:23 PM
> To: Nick Foster
> Cc: address@hidden
> Subject: Re: [Discuss-gnuradio] digital_correlate_access_code_bb
>
> 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
>
> _______________________________________________
> Discuss-gnuradio mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/discuss-gnuradio