[chirp_devel] Re: Patch - [FT-60] Feature Request: Memory Bank Support #1945

[stickpatrick at gmail.com]

I tested your patch with a manual merge fix & added an exception to pass the tests.  I can't tell any difference performance wise, so I think we should check in your shorter version without the cache since it's a bit easier to read.


If you have the other 2 patches too, that's great!  I managed to get 1-999 working, but didn't include a fix for 000.


Exception I added to remove_memory_from_mapping:

 if self._radio._memobj.banks[bank.index].memory[number].get_bits(mask) != mask:
     raise Exception("Memory {num} is " +
                     "not in bank {bank}".format(num=memory.number,
     bank=bank))




Here's what I was considering as a bank UI fix, but I wasn't confident it wouldn't break banks for another radio with memories labeled from ‘0’:


diff -r 9bc3cda2e2eb chirpui/bankedit.py
--- a/chirpui/bankedit.py       Sat Jan 10 16:05:27 2015 -0800
+++ b/chirpui/bankedit.py       Sat Jan 10 17:41:00 2015 -0800
@@ -156,7 +156,7 @@
             iter = self._store.iter_next(iter)


         index_bounds = self._model.get_index_bounds()
-        num_indexes = index_bounds[1] - index_bounds[0]
+        num_indexes = index_bounds[1] - index_bounds[0] + 1 # should be inclusi
ve
         indexes.sort()
         for i in range(0, num_indexes):
             if i not in indexes:
@@ -335,7 +335,9 @@
         sw.add(self._view)
         self._view.show()


-        for i in range(*self._rf.memory_bounds):
+        lowerbound = self._rf.memory_bounds[0]
+        upperbound = self._rf.memory_bounds[1] + 1 # needs to be memory_bounds[
1] inclusive
+        for i in range(lowerbound, upperbound):
             iter = self._store.append()
             self._store.set(iter,
                             self.C_FILLED, False,
@@ -372,7 +374,9 @@
         self.rthread.submit(job)


     def refresh_all_memories(self):
-        for i in range(*self._rf.memory_bounds):
+        lowerbound = self._rf.memory_bounds[0]
+        upperbound = self._rf.memory_bounds[1] + 1 # needs to be memory_bounds[
1] inclusive
+        for i in range(lowerbound,upperbound):
             self.refresh_memory(i)


     def refresh_mappings(self, and_memories=False):

 








Sent from Windows Mail





From: Kosta Arvanitis
Sent: ‎Saturday‎, ‎January‎ ‎10‎, ‎2015 ‎5‎:‎06‎ ‎PM
To: chirp_devel at intrepid.danplanet.com





I looked over both patches, the look very similar.  


There is also a known bug with the memory bounds on the FT-60 module; it is incorrectly specified to not include the 1000th memory bank.  I have a separate fix for this as well.


In addition there is a bug with the BankEditor which is not inclusive of the memory bounds as well, so it also does not display the 1000th memory bank.  I have a separate fix for this as well.


If its ok with Scott, given there is little diff. between the patches I would prefer to apply the one I submitted last week so that I can also submit the two subsequent patches as well which depend on it.  



-kosta



________________________________
> From: stickpatrick at gmail.com 
> To: chirp_devel at intrepid.danplanet.com 
> Date: Sat, 10 Jan 2015 23:44:28 +0000 
> Subject: [chirp_devel] Re: Patch - [FT-60] Feature Request: Memory Bank 
> Support #1945 
> 
> I'm looking at Kosta's now. I took the same approach as the FT857 and 
> lifted a lot of that code including the cache making it a bit harder to 
> read. I’ll see if there's a perf difference or not. 
> 
> Dan - do you have any intention of using chirp_common.MTOBankModel in 
> the future? Right now it's equivalent to chirp_common.BankModel but 
> seems to be a better description of what's in the FT60 since any memory 
> can be stored in 1 or more banks. 
> 
> Sent from Windows Mail 
> 
> From: Dan Smith<mailto:dsmith at danplanet.com> 
> Sent: ‎Saturday‎, ‎January‎ ‎10‎, ‎2015 ‎3‎:‎43‎ ‎PM 
> To: chirp_devel at intrepid.danplanet.com<mailto:chirp_devel at intrepid.danplanet.com> 
> 
>> Can you guys look at each other's patches and decide which should be 
>> applied? 
> 
> I applied Kosta's patch and it's failing one of the bank tests (see the 
> test report from a few minutes ago). 
> 
> So, I'll revert it and let you guys work out which one should go in. 
> Note that you should be able to run the tests locally and verify that 
> the bank model(s) meet the requirements first. 
> 
> Thanks! 
> 
> --Dan 
> 
> 
> _______________________________________________ chirp_devel mailing 
> list chirp_devel at intrepid.danplanet.com 
> http://intrepid.danplanet.com/mailman/listinfo/chirp_devel Developer 
> docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers             
_______________________________________________
chirp_devel mailing list
chirp_devel at intrepid.danplanet.com
http://intrepid.danplanet.com/mailman/listinfo/chirp_devel
Developer docs: http://chirp.danplanet.com/projects/chirp/wiki/Developers
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20150111/1b56078f/attachment-0001.html