Bug 129076 - Facilities to change max up/down speed from systray icon
Summary: Facilities to change max up/down speed from systray icon
Status: RESOLVED FIXED
Alias: None
Product: ktorrent
Classification: Applications
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: Joris Guisson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-06-13 02:41 UTC by Eike Hein
Modified: 2006-06-15 23:07 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments
add 2 popup menu to select Max rate (5.25 KB, patch)
2006-06-13 14:40 UTC, khrothgar
Details
version 2 (4.94 KB, patch)
2006-06-13 16:18 UTC, khrothgar
Details
add 2 popup menu to select Max rate (5.11 KB, patch)
2006-06-14 12:23 UTC, khrothgar
Details
Active encoding highlight in Kate's encoding selection (2.89 KB, image/png)
2006-06-14 13:41 UTC, Eike Hein
Details
add 2 popup menu to select Max rate (5.18 KB, patch)
2006-06-14 14:12 UTC, khrothgar
Details
add 2 popup menu to select Max rate (5.24 KB, patch)
2006-06-14 16:28 UTC, khrothgar
Details
add 2 popup menu to select Max rate (5.35 KB, patch)
2006-06-15 09:18 UTC, khrothgar
Details
add 2 popup menu to select Max rate (5.73 KB, patch)
2006-06-15 11:35 UTC, khrothgar
Details
add 2 popup menu to select Max rate (6.01 KB, patch)
2006-06-15 18:12 UTC, khrothgar
Details
KTorrent upload speed limit popup menu on 800x600 (16.84 KB, image/png)
2006-06-15 20:43 UTC, Eike Hein
Details
KTorrent download speed limit popup menu on 800x600 (16.52 KB, image/png)
2006-06-15 20:43 UTC, Eike Hein
Details
menu splitted (151.18 KB, image/png)
2006-06-15 21:04 UTC, khrothgar
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Eike Hein 2006-06-13 02:41:28 UTC
Version:           2.0_beta1 (using KDE KDE 3.5.3)
Installed from:    Compiled From Sources

Depending on the environment it's run in, the upload/download speed limits of the client may need to adapt fairly quickly and regularly to changed circumstances. It would be convenient to have control facilities for the two settings in the systray icon's popup menu, as seen in some competing applications (e.g. Azureus).
Comment 1 khrothgar 2006-06-13 14:40:36 UTC
Created attachment 16580 [details]
add 2 popup menu to select Max rate

i have already posted this patch in ktorrent forum
Comment 2 Ivan Vasic 2006-06-13 15:59:29 UTC
Patch seems OK, but I cannot set upload limit lower than 5KB/s. Maybe you should change that.

And could you create patch with

cd your_local_ktorrent_copy/
svn diff > patchname.patch

since it's much easier to apply it. This way I had to manually enter filenames...

BTW, thanks for the patch ;)
Comment 3 Eike Hein 2006-06-13 16:14:37 UTC
Anything below 5 KB/s probably won't make much of a dent in most BitTorrent user's connections these days, so I wouldn't go too nuts with the resolution to keep the menu managable.

In Azureus, the selection seems to algorithmically offer a certain number of choices situated around the limits set in the main preferences dialog.
Comment 4 khrothgar 2006-06-13 16:18:47 UTC
Created attachment 16581 [details]
version 2

now should work with value <5 too
Comment 5 Eike Hein 2006-06-13 22:23:37 UTC
The patch has a few problems, IMHO:

1. It doesn't communicate the active speed limits. They should be highlighted with a small bullet next to them or a bold type face.

2. The steppings are a bit weird. When I set upload to 40, it offers "40, 45, 48, 49, 50, ..." for example. 48 and 49 are overdoing it - "40, 45, 50" would be quite enough.

3. The "max rate in kb/s" sub-popup heading isn't particularly elegant English. I'd opt for "Upload in kb/s" and "Download in kb/s" or, if it's supposed to be generic, "Speed limit in kb/s".

Otherwise, definitely a great addition to the systray's usefulness.
Comment 6 Ivan Vasic 2006-06-13 23:05:41 UTC
>1. It doesn't communicate the active speed limits. They should be highlighted with a small bullet next to them or a bold type face.
 
I agree with this.

>2. The steppings are a bit weird. When I set upload to 40, it offers "40, 45, 48, 49, 50, ..." for example. 48 and 49 are overdoing it - "40, 45, 50" would be quite enough.

I'm not quite sure what algorithm he used, but eventually you could always select desired value by choosing the nearest one and then redoing the whole process. I never had to do it more than twice myself.

>3. The "max rate in kb/s" sub-popup heading isn't particularly elegant English. I'd opt for "Upload in kb/s" and "Download in kb/s" or, if it's supposed to be generic, "Speed limit in kb/s". 

I was about to change this with my commit (which I haven't done yet) along with couple of other minor fixes.

So khrothgar, will you make the modifications with your code or should I commit this patch first?
Comment 7 Eike Hein 2006-06-14 00:17:48 UTC
> I'm not quite sure what algorithm he used, but eventually you could always select desired value by choosing the nearest one and then redoing the whole process. I never had to do it more than twice myself. 

You misunderstood my point there. IMHO, the purpose of accessing those settings from the systray icon's popup menu is speed, and speedy navigation of an UI is accomplished by simplicity. Hence, 5 KB/s increments are enough especially around a set value like 40 KB/s - and odd numbers like "48" and "49" are particularly awkward. It looks like a bug in the algorithm.

As for your comment, someone who's willing to use the systray icon facilities twice to set a very specific value can just as easily click "Configure" in the same menu.
Comment 8 Ivan Vasic 2006-06-14 00:37:57 UTC
Well, you're right I admit.
Comment 9 khrothgar 2006-06-14 12:23:21 UTC
Created attachment 16596 [details]
add 2 popup menu to select Max rate

fixes to problems signaled by Eike Hein

Sorry for my poor english.
Comment 10 Eike Hein 2006-06-14 13:40:38 UTC
The steppings and wording look good now, my only remaining nitpick would be the "->" used to highlight the currently set speed limits. That's pretty non-standard as far as KDE UI designs go. Maybe the ToggleAcction design use by Konqueror, Kate, Konversation & Co to select the document/channel encoding in a popup menu could be used. I'll attach a shot. 
Comment 11 Eike Hein 2006-06-14 13:41:27 UTC
Created attachment 16597 [details]
Active encoding highlight in Kate's encoding selection
Comment 12 khrothgar 2006-06-14 14:12:25 UTC
Created attachment 16598 [details]
add 2 popup menu to select Max rate

now have checkable item
thanks for the suggestion
Comment 13 Eike Hein 2006-06-14 14:36:50 UTC
Looks great now. Thanks for your effort!
Comment 14 Eike Hein 2006-06-14 14:42:20 UTC
Oops, yet another nitpick I'm afraid: The algorithm doesn't cope very well when a limit is set to "Unlimited".

When I have upload on 40, it offers reasonable choices below and above 40. When it's set to "Unlimited", however, it offers 350-650, which is way outside the range that is useful to me. "Unlimited" should probably be treated as a special case that offers some smaller choices as well.
Comment 15 khrothgar 2006-06-14 14:50:30 UTC
if it will offer 10-200 is ok?
same matter for download or it's ok with 350-650 range?
Comment 16 Eike Hein 2006-06-14 15:10:17 UTC
Hmm, tough choice. 

Over here in Germany, most broadband connections are asynchronous DSL, with 2 or 6 Mbit/s download capacity, but significantly less upload capacity. For example, I can do up to 70 KB/s up and 700 KB/s down. 

My upload limit us usually set to 40 KB/s. When I'm not relying on speedy web access, I sometimes set it to 50, or even unlimited over night, when I'm seeding anything. Occassionally I will also lower it to 20 or 25, when I have another P2P app running in parallel. My download speed is usually set to unlimited.

10-200 in the upload selection and 350-650 in the download selection would be ideal for me, but I have no idea what's common in KTorrent's other "markets".

Azureus appears to offer 5-200 (up) and 75-475 (down) in the "Unlimited" case, which would be similar. Assuming they've gone through a couple of iterations on their menu as well, 10-200 up would probably work out fine, but the download selection may have to offer some lower choices as well.


BTW, another suggestion: When one of the limits is set to an odd number like, say "37", the menu will offer rates like "42, 47 ...". Maybe it would be nicer to round any odd number to the next nicer one, i.e. 37 to 35, internally. So, 37 would still be shown as currently set, but it would offer 40, 45 etc. above it. 
Comment 17 khrothgar 2006-06-14 16:22:40 UTC
I have a problem. when i set max speed from systray and close ktorrent change doesn't persist.
seems that Settings::setMaxUploadRate(v) set only a variable in Settings class without apply it to the system.
Comment 18 khrothgar 2006-06-14 16:28:03 UTC
Created attachment 16602 [details]
add 2 popup menu to select Max rate
Comment 19 Eike Hein 2006-06-14 16:52:39 UTC
Hm, small problem with the rounding logic, IMHO: If it's set to "39", it should probably internally round to 40 and replace that in the list, not 35.
Comment 20 Eike Hein 2006-06-14 16:55:12 UTC
(Basically, 36-37 -> 35; 38-39 -> 40.)
Comment 21 khrothgar 2006-06-15 09:18:49 UTC
Created attachment 16620 [details]
add 2 popup menu to select Max rate

i have added Settings::writeConfig() in SetMaxRate::rateSelected so if you
close ktorrent before open Configure the change persist
Comment 22 khrothgar 2006-06-15 11:35:18 UTC
Created attachment 16621 [details]
add 2 popup menu to select Max rate

sorry previuos is wrong
Comment 23 Eike Hein 2006-06-15 13:34:54 UTC
The limits are now correctly saved across sessions. It still rounds wrong, though.
Comment 24 khrothgar 2006-06-15 15:54:57 UTC
now if i set max speed manually through configure to "39" why round to 40 in systray popup. i haven't understood the problem
Comment 25 Eike Hein 2006-06-15 16:08:51 UTC
The goal of my suggestion is visual polish :). 

I initally remarked that when you happen to have set your upload rate to 37 (perhaps deliberately, or by clicking in the Configure dialog a few times too often unintentionally), the popup menu would display suggestions like "42" and "47" around it. For various reasons, the human mind works better with even steppings of ten or its half, 5 - i.e., 40 and 45 are nicer to work with in a popup menu.

So, when a value like '37' is selected, the code should internally round to the nearest 'nice' number and replace it in the list. For 37, that's 35. So you'd get a list like:

25
30
[37]
40
45

That's what you've got working right now, which is great. The problem, then, is that you currently always round down, never up, even when the number suggests otherwise. I.e. currently, you run into this:

30
[39]
40

... when it should be:

35
[39]
45

39 should replade 40, not 35, because 39 is nearer to 40 than 35. 

I know, I'm a pain in the ass - but that's actually the last nitpick I have, I promise ;).
Comment 26 khrothgar 2006-06-15 18:12:30 UTC
Created attachment 16624 [details]
add 2 popup menu to select Max rate
Comment 27 Eike Hein 2006-06-15 18:53:54 UTC
Great work :). I think from a user experience point of view the patch is really solid now. Thank you for putting up with my suggestions.

Ivan, what do you think? Ready to commit?
Comment 28 khrothgar 2006-06-15 20:20:19 UTC
thank you for the suggestion
Comment 29 Joris Guisson 2006-06-15 20:35:40 UTC
I think we can commit, allthough should we not make the menu a bit smaller ? How does this look in a 800x600 resolution ? I think it will probably be to big to fit fully on the screen.
Comment 30 Eike Hein 2006-06-15 20:43:06 UTC
Created attachment 16627 [details]
KTorrent upload speed limit popup menu on 800x600
Comment 31 Eike Hein 2006-06-15 20:43:33 UTC
Created attachment 16628 [details]
KTorrent download speed limit popup menu on 800x600
Comment 32 Eike Hein 2006-06-15 20:44:40 UTC
Should fit even with fairly large font sizes, I think.

(Notably, KTorrent itself doesn't fit on a 800x600 screen. It's impossible to "Maximize" it as it doesn't fit maximized to 800x600.)
Comment 33 Joris Guisson 2006-06-15 20:53:51 UTC
Yeah, I know we plan on fixing this, but we have to be sure that any further additions will fit in 800x600, otherwise we would have to fix that to.
Comment 34 khrothgar 2006-06-15 21:00:06 UTC
we can reduce the number of choice however if it will be too big create it self a new column
Comment 35 Eike Hein 2006-06-15 21:01:48 UTC
Believe me, I know the problem ;). Over in the Konversation camp, the main culprit is the prefs dialog, which is quite telling.

Anyhow, I think the menu as it exists now should just about fit even with 12pt fonts - and it's not like it breaks when it doesn't fit; the menu widget goes into multi-column mode in that event. The large encoding selections mentioned above exhibit that trait on many screens, for example.
Comment 36 khrothgar 2006-06-15 21:04:26 UTC
Created attachment 16629 [details]
menu splitted
Comment 37 Ivan Vasic 2006-06-15 22:48:56 UTC
SVN commit 551869 by ivasic:

ADD: Added popup menu to system tray to quickly select transfer rate limits. Big thanks to Khrothgar for providing the patch and Eike Hein for suggestions.

BUG: 129076

 M  +9 -0      ktorrent.cpp  
 M  +3 -0      ktorrent.h  
 M  +10 -0     ktorrentcore.cpp  
 M  +3 -1      ktorrentcore.h  
 M  +110 -18   trayicon.cpp  
 M  +38 -17    trayicon.h  
Comment 38 Eike Hein 2006-06-15 23:07:29 UTC
Great stuff; thanks to all involved.