Thanks for your reply!

On 2013-02-19 10:14, Tom Hayward wrote:
... 
A new file is a change to Chirp. After adding the new file, Chirp is
not the same as it was before. But that's not the point here...

I understand that.  In CVS, to add a new file, I do "cvs add <filename".  I presume in Mercurial I do "hg add chirp\id51py" (which I did), which would be added to the repository by a commit.  However, if I do the commit, Chirp's web site seems to imply that creates a fork that creates problems, so I haven't done that.


... "hg status -mar" does nothing to attribute the changes to you, or label them with an appropriate commit message.

I understand that "hg status -mar" is the same as "cvs upd";  ie, just lists the files that have changed or been added, and will be committed on the next commit.


Our goal here isn't just to add new lines of code, but to document who added them and why, so that future readers of the code can ask "what was the guy who wrote this thinking??" and have both the explanation (commit message) and name/email of the guy who wrote it. If we find that your change introduces a regression, we are also easily able to search for commit messages containing "id?1" and revert each as needed until the regression disappears. 
hg diff:

For this all to work, you must use a different command, "hg qnew ..."
to create your patch. You will be prompted to enter a commit message,
and when you export it ("hg export tip") your name and email address
will be automatically be added to the header. The commands to
accomplish this are all listed here:

http://chirp.danplanet.com/projects/chirp/wiki/DevelopersProcess#Soft-committing-a-Change

So far, I've been sent that link THREE TIMES.  Each time I have read it IN DETAIL.  I don't want to start an argument here, BUT IT IS FAR FROM CLEAR.  The Mercurial website documentation is poorer;  I had to Google to find the format of the configuration file (named ".hg/hgrc", not ".hg/config" as the Chirp website indicates).  Note that I have worked with several source control systems over the forty years I've been developing, so I have a bit of a clue.

EG:

  1. The first time I ran "hg export tip", it showed changes (by Dan) to files that I hadn't touched.  What's that about?
  2. Subsequent invocations return my changes to two files.  These two files need to be two separate patches (two separate bug #s).
  3. What the hell is "tip" ???
  4. OK, I did "hg qnew -l chirp\id51.py -ef <comment>".  Then I removed it with "hg qpop".  Somewhere in these few steps my changes (including all of id51.py) were deleted.   Is that what you mean by "minimal learning curve" below?  Fortunately (that means having some experience with the batch of new SCM packages out there), I had a complete backup elsewhere.
  5. I just now decided to have multiple backups of my changes.  Perhaps I'll use CVS internally ...
  6. Another "hg qpop" also removed a "patch" named "help";  I'll let you guess as to how that got there (grin).

When using "hg q.." commands, your changes are not permanent. You can
send a work-in-progress patch (see Sending a Change), ask some
questions, modify your code, then use "hg qref" to update the patch.

A little bit of record keeping is necessary for managing a large
codebase, and I think you'll find the learning curve for hg mq is
minimal and saves you effort in the long run.

Tom KD7LXL

<rant>

So far this has been all downhill.  I've spent far more time screwing around with the submission process, that I did to create id51.py (and make proposed changes to id31.py).  I don't have/need/want an ID-31A.  I have an ID-51A, and the free Icom CS-51 software is sufficient for my needs (including the import of some 250 memories via .CSV files).  Frankly, it's less buggy as well.

I thought, "Chirp lacks ID-51 support, and I have one;  let's see if I can help".  That's my sole motivation.  Just as changing DStarCom to open source so that Chirp could use some of that code, was to help (see Dan's blog on my code).  The changes I have to id31.py are because (having cloned id51.py from it) I think there are errors in the memory layout, and it seems reasonable to help fix that as well.

We're all volunteers here, and I don't expect anyone to jump through hoops for me.  On the other hand, I'm retired, and have plenty of spare time to contribute to useful efforts.  Of course, I have MANY choices for how I spend that time (that includes being paid to fly small airplanes in wonderful weather), and screwing around with yet another inadequately documented tool is not high on my list.

I don't know if you have read the book "Psychology of Computer Programming" by Gerald Weinberg.  Written in 1972, it's considered a classic.  He reported that back then, someone was inventing a new computer language about once a week.  This industry is filled with people that "have to" create new ("vanity") languages or tools, with very marginal additional utility (or worse) between them.  Remember "BitKeeper"?  That was the SCM rage a couple years ago.  No one these days counts the productivity cost of learning new tools (for Microsoft, that means new versions of productive software, with the main change being useless changes to the GUI) that are not adequately documented.

</rant>

So, I'll try once more:

   hg qnew -l chirp\id51.py -ef "[id51] Add support for Icom ID-51"

I'm supposed to enter the bug # (553), but "qnew" won't take a "#" on the command line, and up pops a NotePad window with the changed file (??? I just closed it).  So, no info on that.  What follows is the output from the next command. I sure don't see my comment (above), and I sure thought that almost all of the lines would be prefixed by a "+".  So,, my guess is that this will be unacceptable as well.  I'm sure not going to submit any changes to id31.py until I've sorted this out.

   hg export tip

# HG changeset patch
# User Dean Gibson <data@ae7q.net>
# Date 1361307603 28800
# Node ID b94ce8ef429fb5bf5750d96341c6d38354837b57
# Parent  5528bdcdc34e5966d5b7f0dbb6859f700b5f9366
# Copyright 2012 Dan Smith <dsmith@danplanet.com>
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# 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

MEM_FORMAT = """
struct {
  u24 freq;
  u16 offset;
  u16 rtone:6,
      ctone:6,
      unknown2:1,
      mode:3;
  u8 dtcs;
  u8 tune_step:4,
     unknown5:4;
  u8 unknown4;
  u8 tmode:4,
     duplex:2,
     dtcs_polarity:2;
  char name[16];
  u8 unknown13;
  u8 urcall[7];
  u8 rpt1call[7];
  u8 rpt2call[7];
} memory[500];

#seekto 0x6A40;
u8 used_flags[70];

#seekto 0x6A86;
u8 skip_flags[69];

#seekto 0x6ACB;
u8 pskp_flags[69];

#seekto 0x6B40;
struct {
  u8 bank;
  u8 index;
} banks[500];

#seekto 0x6FD0;
struct {
  char name[16];
} bank_names[26];

#seekto 0xA8C0;
struct {
  u24 freq;
  u16 offset;
  u8 unknown1[3];
  u8 call[7];
  char name[16];
  char subname[8];
  u8 unknown3[10];
} repeaters[700];

#seekto 0x1384E;
struct {
  u8 call[7];
} rptcall[700];

#seekto 0x14E60;
struct {
  char call[8];
  char tag[4];
} mycall[6];

#seekto 0x14EA8;
struct {
  char call[8];
} urcall[200];

"""

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):
    """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.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