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 -----Original Message----- From: discuss-gnuradio-bounces+sean.nowlan=gtri.gatech....@gnu.org [mailto:discuss-gnuradio-bounces+sean.nowlan=gtri.gatech....@gnu.org] On Behalf Of Tom Rondeau Sent: Monday, April 09, 2012 5:23 PM To: Nick Foster Cc: Discuss-gnuradio@gnu.org Subject: Re: [Discuss-gnuradio] digital_correlate_access_code_bb On Mon, Apr 9, 2012 at 2:02 PM, Nick Foster <n...@ettus.com> wrote: > On Mon, Apr 9, 2012 at 10:48 AM, Marcus D. Leech <mle...@ripnet.com> wrote: >> >> On 04/09/2012 01:38 PM, Tom Rondeau wrote: >>> >>> On Sat, Apr 7, 2012 at 10:12 PM, Marcus D. Leech<mle...@ripnet.com> >>> 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 Discuss-gnuradio@gnu.org https://lists.gnu.org/mailman/listinfo/discuss-gnuradio _______________________________________________ Discuss-gnuradio mailing list Discuss-gnuradio@gnu.org https://lists.gnu.org/mailman/listinfo/discuss-gnuradio
