-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Reworked Bandwidth Scheduler #9203
base: master
Are you sure you want to change the base?
Reworked Bandwidth Scheduler #9203
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as off-topic.
This comment was marked as off-topic.
Unless I am mistaken github allows other people to push to your branch once you have opened a PR based on it. So your branch can be the feature branch you're asking for. |
This comment was marked as outdated.
This comment was marked as outdated.
12f3361
to
d8adee2
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@FurkanKambay I quickly skimmed your posts. I didn't read any of the code. Here are my thoughts and suggestions. If something is interfering with what you have already decided/implemented disregard it. I have given thought to this issue a long time ago, but I lack the time and motivation to actually code it. Basically we let the user define a specific Date-Time(timestamp) and what happens(action) when that Date-Time comes. So the UI just represents a list of timestamps and their associated actions. In the 1st iteration of the implementation of the scheduler the only allowed action is to enable/disable alternative speed limits. A few words on how to approach this PR.
The core
GUI and WebUI The above is just in abstract. When actually coding it you might need to make modifications. I hope I didn't give you headaches. |
Does it read which weekday should be first, and the time and date formats from the system? |
Why does this matter? |
Because you are supposed to support the user's format. I, as a European, don't want to use the American format just like how an American doesn't want to use the European format. (If they want to use a different format, they can change it in the regional settings). |
The UI representation is independent of the internal one. The UI should display the formats in the current locale anyway. |
Can we expect this on an upcoming beta release? I am looking forward to being able to pause torrents during the day instead of the pseudo-pause of 1kbps and still connected to the swarm. This has a good use case for Sonarr and Radarr users that need constant client uptime (for torrent adding) but want scheduled upload/download downtime. Otherwise I would just close the client during the day. |
I'd also like an update. I know the PR is a bit large so let me know if there's anything you need to know. I manually tested the app with the latest appveyor build on a clean Windows 11, and it works just fine. There are only minor visual bugs that have already been discussed here, and as I said, I have no time or motivation to fix them, but also they're small enough to at least make it into the beta in my opinion. (ps, I'll squash my commits once it's ready for merging) |
Is there anything I can do right now? I will be mostly unavailable until the end of the year. If there are any concerns or bugs, I might be able to address them this month. |
Maybe this has been discussed before but... being unable to set e.g. 10 PM -2 AM (next day) is not user friendly.
|
I removed 'advanced' this time, sorry I forgot.
I could see someone expecting it to work like that, but I think I don't have the time or motivation to implement it now. |
This is exactly what I am looking for. Any news? |
@glassez @FranciscoPombal @xavier2k6 Do you have a guess as to when this can be reviewed? Or could I get some preliminary feedback based on the information and screenshots I provided in the OP? I will probably have time to work on this from January. Some questions:
Also, some testing would be very much appreciated from anyone who'd like to see this feature. todo for me
|
Can the PR be rebased? then we can get testers to test it |
I honestly tried to review it, as promised earlier. I've started this several times, trying to come in from different ends. But unfortunately it was not successful. In the end, I came to the conclusion that it does not make sense to review the entire PR, making comments in each specific place, but first you need to eliminate its fundamental shortcomings, which are blocking.
I hope the above was at least somewhat understandable. P.S. Such long comments are uncharacteristic for me. I prefer to avoid discussions in which I need to read and (especially) to write such comments. I hope that further discussion of this topic will not require such comments. |
@glassez Thanks for the comprehensive explanation. I understand the problems; I'll start working on those probably in January. @luzpaz There are a bunch of conflicts to go through so rebasing will take time but the build artifacts can be downloaded from the CI runs right now. However, testing can wait now since I have more work to do. |
Still hoping too see this merged into master. |
For those who are wondering, I started implementing this feature in #9203 years ago but some issues need to be resolved before the maintainers can even review it. Unfortunately, I don't have the time to dedicate more time to this right now (plus I don't use qBittorrent as much anymore)... I wanted to continue working on it by last January but life happened. If any dev would like to pick up where I left off or start from scratch they're welcome to... (I don't know if the PR should be closed or just marked as draft, I'm leaving that to the maintainers. I guess if I ever want to come back and finish the feature I'll just ask to reopen?) |
This implements the advanced bandwidth scheduler described in #8804 but without the fancy grid UI.
Explanation & features implemented
ScheduleDay
andScheduleEntry
.Paused
state at current time (from the new scheduler, explained below),Pause
option that pauses or resumes the nativelt::session
. This is currently being prioritized more than the alternative limits but that can be changed.importLegacyScheduler
method)start time
,end time
,download speed
,upload speed
, andpause
.session paused
state is displayed as[Paused]
on the status bar.Screenshots