On Tue, Apr 3, 2012 at 18:53, Dan Smith dsmith@danplanet.com wrote:
I'll send a patch to the list in a minute that will enable doing so, and convert the three cases there to use match_model() instead. Let me know what you think about doing it that way instead.
Looks good. I'll get this patch updated to use it.
+class HMKRadio(chirp_common.CloneModeRadio):
I was thinking you would inherit from CSVRadio and hopefully eliminate the need to duplicate all that work. Was there some reason this didn't work out?
I was hoping to inherit CSVRadio, but CSVRadio inherits IcomDstarSupport and I don't want that. If I inherit from CSVRadio, is there a way to disable the Dstar stuff?
At first when I supported File > Open, I was getting a Dstar tab I didn't want. Now that it's import-only, this might not be an issue. I just don't want to see Dstar columns/features.
I don't think you need _blank() if you're not going to support editing, right? If you inherit it from CSVRadio, it's one thing, but I think it'll confuse a future developer if it's here for no reason.
Indeed. That is leftover copypasta that I meant to remove.
- def __init__(self, pipe):
- chirp_common.CloneModeRadio.__init__(self, None)
- self._filename = pipe
- if self._filename and os.path.exists(self._filename):
- self.load()
- else:
- self._blank()
It's an error if we're instantiated without a filename.
Added to my list of fixes.
- def get_features(self):
If you inherit from CSVRadio, then these are fine, but I think they are probably unnecessary if you're import-only, right?
Ok, are you saying get_features() is not used during import? I can remove it if it's not used.
- def _parse_quoted_line(self, line):
- line = line.replace("\n", "")
- line = line.replace("\r", "")
- line = line.replace('"', "")
- return line.split(",")
This is dead code from the CSV driver. Lets remove it from there and not replicate it here :)
Ok
- def _parse_csv_data_line(self, headers, line):
- mem = chirp_common.Memory()
- odd_split = False
- for header, (typ, attr) in self.ATTR_MAP.items():
- try:
- val = self._get_datum_by_header(headers, line, header)
- if not val and typ == int:
- val = None
- elif attr == "duplex":
- val = typ(self.DUPLEX_MAP[val])
- if val == "split":
- odd_split = True
Hmm, what do the Tx Freq. and Offset columns look like in the two cases? Seems like we should be able to do better than this.
In this loop we only have access to one column at a time, so I am saving the vars odd_split and tx_freq to a broader scope and making the final assignment outside the loop. What exactly are you suggesting?
- elif attr == "skip":
- val = typ(self.SKIP_MAP[val])
- elif attr == "tmode":
- val = typ(self.TMODE_MAP[val])
Instead of special-casing these, why not use a function like the frequency parsing case? Something like this:
ATTR_MAP = { ... "T/CT/DCS" : (lambda v: self.TMODE_MAP[v], "tmode"), "L.Out" : (lambda v: self.SKIP_MAP[v], "skip"), ... }
That way you can just cover all the cases with this:
You are strong in the ways of Python.
- else:
- val = typ(val)
...like the CSV driver does. Plus, if we can figure out something sneaky for the Offset and Tx Freq. cases, then you could still inherit from CSVRadio with a different ATTR_MAP :)
- def load(self, filename=None):
- if filename is None and self._filename is None:
- raise errors.RadioError("Need a location to load from")
- if filename:
- self._filename = filename
- self._blank()
I think you can remove this.
Ok
- f = file(self._filename, "r")
- for i in range(0, 10):
- f.readline().strip()
Is it always ten lines? Maybe it would be better to probe smartly to chew up everything until the header row?
I have three example hmk files from three different Kenwood programming software and they are all 10 lines. But yes, I think I can do better. The header line always starts with !!Ch, so I can look for that, back up one line, and send that to the csv reader.
- #f.seek(0, 0)
Did you intend to leave this in or take it out?
This needs to be removed. Seeking back to 0 would obviously make my 10-line skip worthless :)
-- Dan Smith www.danplanet.com KK7DS
Tom KD7LXL