[chirp_devel] [PATCH 1 of 3] [id31] Backport ID-51 mode logic to ID-31. Related to #553
# HG changeset patch # User Tom Hayward tom@tomh.us # Date 1361483689 28800 # Node ID 169fa6015eb76d265fc24b2f705fcc40f8bac6e5 # Parent 963296a371c6d65359dccc9d6ebf147ed6955f79 [id31] Backport ID-51 mode logic to ID-31. Related to #553
diff -r 963296a371c6 -r 169fa6015eb7 chirp/id31.py --- a/chirp/id31.py Wed Feb 20 19:44:20 2013 -0800 +++ b/chirp/id31.py Thu Feb 21 13:54:49 2013 -0800 @@ -22,9 +22,7 @@ u16 rtone:6, ctone:6, unknown2:1, - is_dv:1, - unknown2_0:1, - is_narrow:1; + mode:3; u8 dtcs; u8 tune_step:4, unknown5:4; @@ -88,6 +86,7 @@
"""
+MODES = {0: "FM", 1: "NFM", 3: "AM", 5: "DV"} TMODES = ["", "Tone", "TSQL", "TSQL", "DTCS", "DTCS", "TSQL-R", "DTCS-R"] DUPLEX = ["", "-", "+"] DTCS_POLARITY = ["NN", "NR", "RN", "RR"] @@ -198,7 +197,7 @@ rf.has_bank_names = True rf.valid_tmodes = list(TMODES) rf.valid_tuning_steps = sorted(list(TUNING_STEPS)) - rf.valid_modes = ["FM", "NFM", "DV"] + rf.valid_modes = MODES.values() rf.valid_skips = ["", "S", "P"] rf.valid_characters = chirp_common.CHARSET_ASCII rf.valid_name_length = 16 @@ -218,7 +217,7 @@
bit = (1 << (number % 8))
- if _mem.is_dv: + if MODES[int(_mem.mode)] == "DV": mem = chirp_common.DVMemory() else: mem = chirp_common.Memory() @@ -237,16 +236,12 @@ mem.dtcs = chirp_common.DTCS_CODES[_mem.dtcs] mem.dtcs_polarity = DTCS_POLARITY[_mem.dtcs_polarity] mem.tuning_step = TUNING_STEPS[_mem.tune_step] + mem.mode = MODES[int(_mem.mode)]
- if _mem.is_dv: - mem.mode = "DV" + if mem.mode == "DV": mem.dv_urcall = _decode_call(_mem.urcall).rstrip() mem.dv_rpt1call = _decode_call(_mem.rpt1call).rstrip() mem.dv_rpt2call = _decode_call(_mem.rpt2call).rstrip() - elif _mem.is_narrow: - mem.mode = "NFM" - else: - mem.mode = "FM"
if _psk & bit: mem.skip = "P" @@ -279,9 +274,7 @@ _mem.dtcs = chirp_common.DTCS_CODES.index(memory.dtcs) _mem.dtcs_polarity = DTCS_POLARITY.index(memory.dtcs_polarity) _mem.tune_step = TUNING_STEPS.index(memory.tuning_step) - - _mem.is_narrow = memory.mode in ["NFM", "DV"] - _mem.is_dv = memory.mode == "DV" + _mem.mode = next(i for i, mode in MODES.items() if mode == memory.mode)
if isinstance(memory, chirp_common.DVMemory): _mem.urcall = _encode_call(memory.dv_urcall.ljust(8))
# HG changeset patch # User Tom Hayward tom@tomh.us # Date 1361483885 28800 # Node ID 539c7d8f0a972b2780f6707bcf4a41cd2aaa160b # Parent 169fa6015eb76d265fc24b2f705fcc40f8bac6e5 Change chirp_common.Radio to new-style class. Needed for #553
diff -r 169fa6015eb7 -r 539c7d8f0a97 chirp/chirp_common.py --- a/chirp/chirp_common.py Thu Feb 21 13:54:49 2013 -0800 +++ b/chirp/chirp_common.py Thu Feb 21 13:58:05 2013 -0800 @@ -903,7 +903,7 @@ """A fatal error during memory validation""" pass
-class Radio: +class Radio(object): """Base class for all Radio drivers""" BAUD_RATE = 9600 HARDWARE_FLOW = False
# HG changeset patch # User Tom Hayward tom@tomh.us # Date 1361483887 28800 # Node ID b4b4cadeffb199289cf843d54200aee78a2d2f50 # Parent 539c7d8f0a972b2780f6707bcf4a41cd2aaa160b [id51] Remove code duplication. #553
diff -r 539c7d8f0a97 -r b4b4cadeffb1 chirp/id51.py --- a/chirp/id51.py Thu Feb 21 13:58:05 2013 -0800 +++ b/chirp/id51.py Thu Feb 21 13:58:07 2013 -0800 @@ -13,7 +13,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see http://www.gnu.org/licenses/.
-from chirp import directory, icf, bitwise, chirp_common +from chirp import directory, bitwise, id31
MEM_FORMAT = """ struct { @@ -86,238 +86,22 @@
"""
-MODES = [ "FM", "NFM", "AM", "DV" ] -MODE_INDEX = [ 0, 1, 3, 5 ] -TMODES = ["", "Tone", "TSQL", "TSQL", "DTCS", "DTCS", "TSQL-R", "DTCS-R"] -DUPLEX = ["", "-", "+"] -DTCS_POLARITY = ["NN", "NR", "RN", "RR"] -TUNING_STEPS = [5.0, 6.25, 0, 0, 10.0, 12.5, 15.0, 20.0, 25.0, 30.0, 50.0, - 100.0, 125.0, 200.0] - -def _decode_call(_call): - # Why Icom, why? - call = "" - shift = 1 - acc = 0 - for val in _call: - mask = (1 << (shift)) - 1 - call += chr((val >> shift) | acc) - acc = (val & mask) << (7 - shift) - shift += 1 - call += chr(acc) - return call - -def _encode_call(call): - _call = [0x00] * 7 - for i in range(0, 7): - val = ord(call[i]) << (i + 1) - if i > 0: - _call[i-1] |= (val & 0xFF00) >> 8 - _call[i] = val - _call[6] |= (ord(call[7]) & 0x7F) - - return _call - -def _get_freq(_mem): - freq = int(_mem.freq) - offs = int(_mem.offset) - - if freq & 0x00200000: - mult = 6250 - else: - mult = 5000 - - freq &= 0x0003FFFF - - return (freq * mult), (offs * mult) - -def _set_freq(_mem, freq, offset): - if chirp_common.is_fractional_step(freq): - mult = 6250 - flag = 0x00200000 - else: - mult = 5000 - flag = 0x00000000 - - _mem.freq = (freq / mult) | flag - _mem.offset = (offset / mult) - -class ID51Bank(icf.IcomBank): - """A ID-51 Bank""" - def get_name(self): - _banks = self._model._radio._memobj.bank_names - return str(_banks[self.index].name).rstrip() - - def set_name(self, name): - _banks = self._model._radio._memobj.bank_names - _banks[self.index].name = str(name).ljust(16)[:16]
@directory.register -class ID51Radio(icf.IcomCloneModeRadio, chirp_common.IcomDstarSupport): +class ID51Radio(id31.ID31Radio): """Icom ID-51""" MODEL = "ID-51A"
_memsize = 0x1FB40 _model = "\x33\x90\x00\x01" _endframe = "Icom Inc\x2E\x44\x41" - _num_banks = 26 - _bank_class = ID51Bank - _can_hispeed = True
_ranges = [(0x00000, 0x1FB40, 32)]
- def _get_bank(self, loc): - _bank = self._memobj.banks[loc] - if _bank.bank == 0xFF: - return None - else: - return _bank.bank - - def _set_bank(self, loc, bank): - _bank = self._memobj.banks[loc] - if bank is None: - _bank.bank = 0xFF - else: - _bank.bank = bank - - def _get_bank_index(self, loc): - _bank = self._memobj.banks[loc] - return _bank.index - - def _set_bank_index(self, loc, index): - _bank = self._memobj.banks[loc] - _bank.index = index - def get_features(self): - rf = chirp_common.RadioFeatures() - rf.memory_bounds = (0, 499) + rf = super(ID51Radio, self).get_features() rf.valid_bands = [(108000000, 174000000), (400000000, 479000000)] - rf.has_settings = True - rf.has_ctone = True - rf.has_bank_index = True - rf.has_bank_names = True - rf.valid_tmodes = list(TMODES) - rf.valid_tuning_steps = sorted(list(TUNING_STEPS)) - rf.valid_modes = list(MODES) - rf.valid_skips = ["", "S", "P"] - rf.valid_characters = chirp_common.CHARSET_ASCII - rf.valid_name_length = 16 return rf
def process_mmap(self): self._memobj = bitwise.parse(MEM_FORMAT, self._mmap) - - def get_raw_memory(self, number): - return repr(self._memobj.memory[number]) - - def get_memory(self, number): - _mem = self._memobj.memory[number] - _usd = self._memobj.used_flags[number / 8] - _skp = self._memobj.skip_flags[number / 8] - _psk = self._memobj.pskp_flags[number / 8] - - bit = (1 << (number % 8)) - - if MODES[MODE_INDEX.index(_mem.mode)] == "DV": - mem = chirp_common.DVMemory() - else: - mem = chirp_common.Memory() - mem.number = number - - if _usd & bit: - mem.empty = True - return mem - - mem.freq, mem.offset = _get_freq(_mem) - mem.name = str(_mem.name).rstrip() - mem.rtone = chirp_common.TONES[_mem.rtone] - mem.ctone = chirp_common.TONES[_mem.ctone] - mem.tmode = TMODES[_mem.tmode] - mem.duplex = DUPLEX[_mem.duplex] - mem.dtcs = chirp_common.DTCS_CODES[_mem.dtcs] - mem.dtcs_polarity = DTCS_POLARITY[_mem.dtcs_polarity] - mem.tuning_step = TUNING_STEPS[_mem.tune_step] - mem.mode = MODES[MODE_INDEX.index(_mem.mode)] - - if mem.mode == "DV": - mem.dv_urcall = _decode_call(_mem.urcall).rstrip() - mem.dv_rpt1call = _decode_call(_mem.rpt1call).rstrip() - mem.dv_rpt2call = _decode_call(_mem.rpt2call).rstrip() - - if _psk & bit: - mem.skip = "P" - if _skp & bit: - mem.skip = "S" - - return mem - - def set_memory(self, memory): - _mem = self._memobj.memory[memory.number] - _usd = self._memobj.used_flags[memory.number / 8] - _skp = self._memobj.skip_flags[memory.number / 8] - _psk = self._memobj.pskp_flags[memory.number / 8] - - bit = (1 << (memory.number % 8)) - - if memory.empty: - _usd |= bit - self._set_bank(memory.number, None) - return - - _usd &= ~bit - - _set_freq(_mem, memory.freq, memory.offset) - _mem.name = memory.name.ljust(16)[:16] - _mem.rtone = chirp_common.TONES.index(memory.rtone) - _mem.ctone = chirp_common.TONES.index(memory.ctone) - _mem.tmode = TMODES.index(memory.tmode) - _mem.duplex = DUPLEX.index(memory.duplex) - _mem.dtcs = chirp_common.DTCS_CODES.index(memory.dtcs) - _mem.dtcs_polarity = DTCS_POLARITY.index(memory.dtcs_polarity) - _mem.tune_step = TUNING_STEPS.index(memory.tuning_step) - _mem.mode = MODE_INDEX[MODES.index(memory.mode)] - - if isinstance(memory, chirp_common.DVMemory): - _mem.urcall = _encode_call(memory.dv_urcall.ljust(8)) - _mem.rpt1call = _encode_call(memory.dv_rpt1call.ljust(8)) - _mem.rpt2call = _encode_call(memory.dv_rpt2call.ljust(8)) - elif memory.mode == "DV": - raise Exception("BUG") - - if memory.skip == "S": - _skp |= bit - _psk &= ~bit - elif memory.skip == "P": - _skp &= ~bit - _psk |= bit - else: - _skp &= ~bit - _psk &= ~bit - - def get_urcall_list(self): - calls = [] - for i in range(0, 200): - call = str(self._memobj.urcall[i].call) - if call == "CALLSIGN": - call = "" - calls.append(call) - return calls - - def get_mycall_list(self): - calls = [] - for i in range(0, 6): - calls.append(str(self._memobj.mycall[i].call)) - return calls - - def get_repeater_call_list(self): - calls = [] - for rptcall in self._memobj.rptcall: - call = _decode_call(rptcall.call) - if call.rstrip() and not call == "CALLSIGN": - calls.append(call) - for repeater in self._memobj.repeaters: - call = _decode_call(repeater.call) - if call == "CALLSIGN": - call = "" - calls.append(call.rstrip()) - return calls
+MODES = {0: "FM", 1: "NFM", 3: "AM", 5: "DV"}
The ID-31A is UHF only, and thus has no support for AM. That's why I had it the way I had it, although I'm fine making it indexed to better mesh with the ID-51A code (until/unless we find that they used that middle bit for something :)
Thanks a bunch for tackling this!
On 2013-02-21 16:12, Dan Smith wrote:
I see Tom back-ported the relevant differences between the ID-51 and ID-31, which I'd already done (see multiple eMails from Monday) three days ago, but was saving until the ID-51 submission was accepted. I was going to create a separate bug, and submit my ID-31 changes tonight; he's saved me the trouble, and I will discard my changes.
The only thing I would suggest for the future, is that if someone is going to duplicate the advertized effort of someone else, that he/she give notice of doing so, so that the first person doesn't continue working on the same thing. Fortunately, that did not happen in this case.
As you note, the ID-31 does not have "AM" mode. The modes table in the ID-51 and ID-31 follow the modes in the ID-880H:
static char const * const modulations[ 8 ] = { "FM", "FM-N", "?2?", "AM", "AM-N", "DV", "?6?", "?7?" };
Except of course, that the ID-51 & ID-51 radios do not support "AM-N", but the ID-800H, ID-880H, and IC-2820H do (in both the radio menus and the Icom PC software). The tables in the IC-2820H and ID-800H are slightly different, but contain the same modes. Obviously, the ID-31 and ID-51 are using the tables (and other architectural commonalities) from the more modern ID-880H radio.
I'm just curious as to methodology for Chirp development: Do we inherit one radio from another, or create a common "abstract" radio , and then inherit both from that?
-- Dean
ps: Ref your comment in bug 553, Java does not limit you to one class per file.
On Thu, Feb 21, 2013 at 5:16 PM, Dean Gibson AE7Q data@ae7q.net wrote:
Sorry, I missed that you volunteered to do this. Maybe I was distracted by all the whining. We often chat about future development on IRC (freenode #chirp).
This is not so much a Chirp question as a Python question. For Python (and Chirp), the mantra is DRY (Don't Repeat Yourself). This means you attempt to not duplicate any code. If you find yourself duplicating code, maybe a new class or function is needed.
All Chirp radio drivers inherit from chirp_common.Radio, but from there things are subclassed however is needed. For example, I thought the ID-51 would subclass nicely from the ID-31, but now it looks like the valid modes differ enough for the two that a base class, ID-x1, should be used for both so we can tweak each slightly.
Tom KD7LXL
On Thu, Feb 21, 2013 at 4:12 PM, Dan Smith dsmith@danplanet.com wrote:
One issue I see with your ID-31 code is that you never clear the bits for AM. These should never be set, but if we're going to attempt setting the mode we need to set all the relevant bits, even if some of them are zeros. Dean's indexed method does this best, so I backported the concept to ID-31 (but changed the implementation).
If we need to hide AM on ID-31, maybe both radios need to be based on an IDx1Radio so that each can have independent mode lists.
Tom KD7LXL
On 2013-02-21 17:40, Tom Hayward wrote:
If you are going to be working more on id31.py, one of the changes I had made and was going to test, once I got a .ICF file for the ID-31, is that the mycall structure in id31.py should be the same as that in id51.py. Apparently no one ever tested the radio with a callsign tag (or for that matter, with multiple myCall values from the radio).
There are other differences as well, like the number of repeaters (700 vs 750), and if you ever implement "groups" (20 vs 25).
After I made the changes for the ID-51 to Chirp, I created corresponding C++ source files for DStarCom for both radios. I created a class for each radio, and an abstract "IdX1a" class as well, for both to inherit from, most of which was lifted from the ID-880H code. I may go back and see if the ID-880 can inherit from that abstract class as well, so that the radios supporting "DR" mode have a common base, which may be where Icom/Japan firmware development is going). The format (including the your/rpt1/rpt2 callsign packing) of a "channel" is identical among the three radios (and presumably the IC-80AD as well), except for the length of the "label" (8 chars in the ID-880H, 16 in the ID31/51A).
I don't know how your ID-51 changes are going to handle the different memory locations in the ID-51 vs the ID-31, given the apparently fact that the id31.py code which is being inherited, has different addresses for everything except the channel memories. Channels will work the same, but what about other stuff (eg, the other callsign tables)???
-- Dean
On Thu, Feb 21, 2013 at 6:20 PM, Dean Gibson AE7Q data@ae7q.net wrote:
The MEM_FORMAT is basically the only thing that's overloaded. This works because all the names you used in MEM_FORMAT are identical to the names used in the ID-31 driver.
Tom KD7LXL
process_mmap() is where MEM_FORMAT is used to interpret the radio image. Since that is overridden in the subclass but uses the same symbol names, the object that each driver builds will have the same interface, but point to different regions in the actual data. The behavior of the overridden method in the subclass is the same in Java or C++, of course.
Sure, but at the time, I didn't know them to be for AM :)
If we need to hide AM on ID-31, maybe both radios need to be based on an IDx1Radio so that each can have independent mode lists.
Or just make the '51 add AM to the mix in get_features() and use that list for the index in get/set_memory(). Either way.
On 22/02/2013 02:40, Tom Hayward wrote:
If we need to hide AM on ID-31, maybe both radios need to be based on an IDx1Radio so that each can have independent mode lists.
Why not move it and all other radio specific things inside the class so that they are isolated (but you can still override them in child classes)? BTW This is the way I do things in ft8x7 classes.
my two cents.
73 de IZ3GME Marco
participants (4)
-
Dan Smith
-
Dean Gibson AE7Q
-
IZ3GME Marco
-
Tom Hayward