Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new library module fritztam #140

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

new library module fritztam #140

wants to merge 1 commit into from

Conversation

nejewoge
Copy link

library module to interact with integrated telephone answering machine
(TAM)

Hey. I really like your python module and I added some basic support for the telephone answering machine, as I needed that.

If you are interested in adding this to the lib-directory, here is a pull request.

The module is documented in library.rst, so I refer to that file for details.

This is actually my first pull request ever, so if I did something wrong let me know an I will try to fix it.

Of course if you have any questions, I am happy to answer them.

library module to interact with integrated telephone answering machine
(TAM)
@kbr
Copy link
Owner

kbr commented Jan 22, 2022

Thank you for the pull request. I will come to a code review and testing when I find some time for that. So please be patient.

@nejewoge
Copy link
Author

Thank you. Take you time. I am happy to answer any questions you might have.

@@ -287,6 +287,78 @@ FritzStatus API
:members:


FritzTAM
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would avoid abbreviations like this. What about FritzAnsweringMachine ?

internally.
"""
if self._sid_token is None:
sidResponse = self.fc.call_action("DeviceConfig",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use pep8 as coding style, i.e. sid_response. There are more issues like this in the code.

return result

# @property
def tam_list(self):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name it tams or anything else, but don't but a type into the label (like _list here).

@kbr
Copy link
Owner

kbr commented Jul 17, 2022

Hi, in case you are still interested, I've started with a first code review, but need some more time for a more detailed read. Thanks for your contribution.

@nejewoge
Copy link
Author

nejewoge commented Aug 8, 2022

Hi. Yes I am still interested. Thank you.

I will have a look an address your comments. I have just been a bit busy recently.

@Paddy0174
Copy link

Just stumbled in here, coming from Home Assistant and would love to see this implemented as fritzconnection is the underlying library used in HA. :)

Can I do something to resolve this, and if so, what specifically or should I make a new PR to get this implemented?

Thanks!

@nejewoge
Copy link
Author

nejewoge commented Jun 3, 2024

Hey. I must say I lost interest in this. The original project, for which I wanted to use this, did not work out. (And then I had some git-struggles which prevented me from resolving the comments above.) Sorry for that.

So @Paddy0174, feel free to make an new PR. I am happy that this seems to useful for somebody.

@nielsnl68
Copy link

I like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants