-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
Add qbittorrent command, multiple sonarr/radarr instance support, single line commands #169
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still a lot of bugs in this PR:
-
When you Addarr for the first time in a chat and send
start
you should get the following message: "You first need to authorize this chat. What is the password?". Now it doesn't respond tostart
.auth
does get a respond of Addarr, but it's still not possible to authorize. -
Some commands without
/
aren't working anymore. By example: Addarr should also respond tostart
instead of only to/start
-
Something wrong with the labels in config. If I use the default config labels, Addarr returns first a warning:
WARNING - Radarr instance with label 'Movie' not found.
and then an error:requests.exceptions.MissingSchema: Invalid URL 'None': No scheme supplied. Perhaps you meant https://None?
. I assume you're wrongly saving/getting the label to set the instance. -
Once you get the first series/movie it isn't possible to go to the next screen.
Yes, add this
,No, show me the next result
andExecute a new search command
don't work. Onlystop
gives a response. -
/delete
command goes to add
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the flake8
remarks
Thank you for the pull request! A lot of interesting new features and some code improvements are always welcome. Could you just look at my remarks? |
My apologies for the code issues.
The start command is fixed now. I dont understand what authorize issue you are describing, but it will now respond to any of the following way of authorization:
Could you describe your flow to arrive at this errors? I am unable to recreate these issues.
For both sonarr and radarr, all buttons are working for me as expected. am i missing something? can you retry with the latest changes please? |
I can confirm that the authorization flow is now working as it's supposed to be and that Addarr responds to
When I use the config as in
I get an error when I press Movie after the question "Is this a movie or a series?". So I start Addarr with Error block
As I understand, it has something to do with non-existent labels, but I don't think this should cause an error since I only select one of the two options: Movie or Series. When I change my config to the below, Addarr responds after I select Movie. So I guess something is wrongly coded that the labels should have the same name as the options it shows after "Is this a movie or a series?"
The flow I follow to get to this issue is like this: Same happens when selecting |
… is single sonarr/radarr instance
This issue seems to have caused by incorrect parsing of the list. I was not able to replicate this error before because this only happens if there is a single radarr/sonarr instance provided in the config. I have since refactored how it fetches the instances.
Second issue is caused by the conversation handler confusing the state of the conversation when jumping from one function to another like if we have just one instance of sonarr it then it calls the function that shows the search results instead of asking the user to select an instance. In such cases I have found that returning the state that the next step needs after calling the next step function resolves this issue.
|
Added some functions I found useful since I started using Addarr.
List of changes:
Refactored Conversation Handler Entry points and States and their flow
Added qBittorrent command to change speeds
Added single line command support. sending a command waiting for prompts to enter data felt tiring. See updated readme
Refactored tag creation. Default tags will also be created if they do not exist.
Multiple instances of Sonarr/Radarr can be added to config.yaml file as a list using labels as an identifier for each instance. Users will be prompted to choose which instance of sonarr/radarr they want to use when adding titles. See config_example.yaml
Refactored some variable/function names. Use of MovieSerie or SerieMovie is odd. Changed to Media. Would recommend use of "Media" to collectively refer to Series and Movies.
Added feature to create telegram notification profile in radarr/sonarr. Users will be notified on these events:
Addressed minor consistency and labelling issues
Adapted all Series/Movies to support multiple instances
Bug Fixes associated with multi instance support in start, auth and delete commands
Registered bot commands programmatically. Available commands will be shown under Menu button in the bot conversation
Future Implementations
DEV NOTE
Rule of thumb I followed implementing returned states and their handlers is that the function will return state named after what that function is currently asking the user to do. And the state handler for that state will be named after the description of the next step that will be performed when that state is triggered.
For example: if the function asks for the title of a movie the function will return PROMPT_TITLE as the state and in the conversation handler the state handler for that state will be:
IMO this will help future developers following the flow easily.