Project

General

Profile

Bug #1557

Updated by Tom Hayward over 10 years ago

The current code related to memory skipping is quite broken. 
 Both memory locations and Skip/Priority sense are scrambled. 
 There are two issues causing the visible bugs, and a third that 
 is exposed with the developer tools if you go looking for it. 

 Each memory location's scan "skip" property can be set to one of 
 "Skip" (S), "Only" (priority scan, P), or "Off" (default). 
 In the radio's button interface, this is done with Set Mode item 46. 
 See the FT-60 User's Manual pages 37 and 38. 

 This sequence demonstrates the problems with the current code. 
 Initially, skips in all memory locations are off. 
 Using the radio's "set" interface set memory 16 to "Skip". 
 Download into chirp. Chirp thinks memory 12 is P (Only) !! 

 The start of @flags[500]@: flags[500]: 
 <pre>28360: 28360: 00 00 00 40 00 00 00 00</pre> 00 

 Set memory 1 to "Only" = Priority = P. Download into chirp. 
 Chirp thinks memory 3 is "S". 
 <pre> 
 28360: 02 00 00 40 00 00 00 00 
 </pre> 

 Flip them: Set memory 1 to Skip, memory 16 to P. 
 Download. Chirp thinks memory 3 is P and mem 12 is S. 
 Same wrong memories, and consistently reversed sense. 
 <pre> 
 28360: 01 00 00 80 00 00 00 00 
 </pre> 

 Analysis of some other examples and the code have determined: 

 A) The radio doesn't include S/P flags for the Chirp @array@ array element @memory[0]@ 
 memory[0] of array @memory[1000]@. memory[1000]. There are actually only normal 
 memories 1-999. In order to have the array offset be the same 
 as the memory number, Chirp defined the array as starting 16 bytes 
 before it really does in the radio's eeprom, but then functionally 
 ignores fictitious @memory[0]@. memory[0]. This will need to be changed to fill 
 out the memory map, but it doesn't need to be changed to fix this bug. 

 There is actually a memory 000, but it lives at the end, where @memory[1000]@ 
 memory[1000] would be if the array were defined as @memory[1001]@. memory[1001]. 

 B) It looks like a complete bit reversal, both with respect to 
 memory order and S/P flags, within each byte. 

 For byte 28360, @flags[0]@, flags[0], which has S/P flags for Mem1 - Mem4, the 
 correct interpretation of the bits is: 
 <pre> 
 (msb) Mem4P    Mem4S    Mem3P    Mem3S    Mem2P    Mem2S    Mem1P Mem1S (lsb) 
 </pre> 
 

 while Chirp is currently interpreting them as 
 <pre> 
 (msb) Mem0S    Mem0P    Mem1S    Mem1P    Mem2S    Mem2P    Mem3S Mem3P (lsb) 
 </pre> 

 One more example to make the pattern clearer: 
 For byte 28368, @flags[1]@, flags[1], which has S/P flags for Mem5 - Mem8, the 
 correct interpretation of the bits is: 
 <pre>(msb) (msb) Mem8P    Mem8S    Mem7P    Mem7S    Mem6P    Mem6S    Mem5P Mem5S (lsb)</pre> (lsb) 
 while Chirp is currently interpreting them as 
 <pre>(msb) (msb) Mem4S    Mem4P    Mem5S    Mem5P    Mem6S    Mem6P    Mem7S Mem7P (lsb)</pre> (lsb) 

 C) It's not causing any functional bugs I've noticed yet, but there's 
 something else wrong with this code: If there are 999 (or 1000) memories, 
 stored in @memory[1000]@, memory[1000], and two flag bits packed into bytes at four 
 memories per byte, then flags should be @flags[250]@, flags[250], not @flags[500]@. flags[500]. 

 @u8 flags[]@ u8 flags[] starts at 0x6EC8. 500 bytes would make the next byte 0x70BC. 
 250 would make the next byte 0x6FC2. The checksum is at 0x6FC8. 
 Makes the case for 250 being correct pretty strong. 

 This bug can be exposed when using the developer tools in the current code: 
 # 

 Use a current daily build, I'd been using 20131204. 
 # Open up any random 
 FT-60 image file. 
 # Open the Browser tab. 
 # Expand root, then flags. 
 # Scroll down and select @flags[500][255]@. flags[500][255]. Looks normal. So would 0-254. 
 # Select @flags[500][256]@, flags[500][256], again normal. 
 # Select @flags[500][257]@. flags[500][257]. Whoops, we're not in Kansas anymore. 
 # You will then find an exception and stack trace in the debug log. 


 The fix involves: 
 # 1) Changing @flags[500]@ flags[500] to @skipflags[250]@. skipflags[250]. The memory offset is unchanged. 
 The name change is in anticipation of bankflags[] being added, and 
 to avoid confusion with an unrelated variable in the file named flags. 
 # 

 2) Indexing @skipflags[]@ skipflags[] with @[memory [memory - 1]@ 1] rather than @[memory]@. 
 # [memory]. 

 3) Reversing the order of both the elements of @skipflags[]@ skipflags[] and the 
 non-zero elements of @SKIPS[]@. SKIPS[]. 

 Testing the change shows that skips defined in the radio interface 
 are now correctly interpreted by chirp, and editing skips in chirp 
 and uploading results in the expected interpretation by the radio.

Back