kbr / fritzconnection

Python-Tool to communicate with the AVM Fritz!Box by the TR-064 protocol and the AHA-HTTP-Interface
MIT License
322 stars 60 forks source link

Phonebook - same names are merged, although different numbers #55

Open bufemc opened 4 years ago

bufemc commented 4 years ago

As said I maintain a phonebook for cold calls. Sometimes, I will just use similar or even equal names, because it's just gambling 1,2,3.. so e.g. I have these items:

Gewinnspiel ['02212928xxx'] Gewinnspiel ['0892213xxx']

But both methods: contacts = fp.get_all_names(phonebook_id) contacts = fp.get_all_numbers(phonebook_id)

will just print out the latter one:

Gewinnspiel ['0892213xxx']

Like all the same named before are overwritten by the last one. If I change one of the two items in the Fritzbox, e.g. to Gewinnspiel1.. then it works and I get printed them both.

As I created 4 issues now let me be clear: I am a big fan of this tool set! I just want to document any strange behavior, or give incentives what could be improved.

kbr commented 4 years ago

Thanks for reporting! That's because get_all_names() returns a dictionary not assuming that a single phonebook may hold identical names. Since get_all_numbers() works on top of get_all_names() duplicates are also overwritten there. A quick and dirty solution can be to return a list of tuples by inheriting from FritzPhonebook and providing just a get_all_name_numbers()-method:

    def get_all_name_numbers(self, id):
        url = self.phonebook_info(id)['url']
        self._read_phonebook(url)
        return [
            (contact.name, contact.numbers)
            for contact in self.phonebook.contacts
        ]

May be that's a candidate for one of the next releases to add it to FritzPhonebook directly.

bufemc commented 4 years ago

Jup, I tried, and that solved it. Thanks!

bufemc commented 4 years ago

Hi kbr, I plan to include fritzconnection soon into my project ( https://github.com/bufemc/a1-fritzbox ) to use the phonebook and add entries (see my other issue, I already provided a code snippet to do so).

I had the idea that there is a solution where you can still use a dictionary: instead of name: { details } .. you could use first found number: { details } .. "normally" there should be only one entry per number. I just wanted to share my thoughts, and it's still not perfect. Things to consider:

Otherwise a simple list should do the job (each item would be a dict). I guess most lists are not that big, so the effort should not become an issue ( https://wiki.python.org/moin/TimeComplexity ).

    def get_all_contacts(self, id, keep_internals=True):
        """
        Get a list of contacts for the phonebook with `id`. Optionally
        remove internal numbers like 'Wecker' by keep_internals=False.
        """
        url = self.phonebook_info(id)['url']
        self._read_phonebook(url)
        return [contact for contact in self.phonebook.contacts if
                keep_internals or not contact.numbers[0].startswith('**')]
bufemc commented 4 years ago

I created a PR ( https://github.com/kbr/fritzconnection/pull/56 ) with this new "master method" (see above), and tried to be backward compatible, so the old methods

get_all_contacts get_all_names

still have the same behavior. Except that (as they use this method) if there are duplicated names in the phonebook, the first entry is returned as "name", 2nd "name_" and 3rd "name__" etc. It's maybe not the best solution but easier to handle than "name", "name (2)", "name (3)".

There is a new, but optional parameter (no worries, default to previous behavior): keep_internals

which just removes internal entries like (German) "Wecker", "Sammelruf" and so on.

So this PR fixes not only this issue ( 55 ), but also https://github.com/kbr/fritzconnection/issues/53


Further thoughts: For now get_all_contacts is the only method returning (/exposing) also the contact.uniqueid (missing in both other methods). Why is this important? If you want to update an entry in the phonebook (in the future), you should use this id as reference only. Example:

You rename the contact with (modified by code above) name="Gewinnspiel_" to name="Gewinnspiel Hampelmann-Verlag", then use something (future enhancement) like

update_phonebook_contact(phonebook_id, contact)

which just and only uses the contact.uniqueid to "find" the meant record in the phonebook.