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

[Stats] Integrated TotalTraffic plugin #367

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

Conversation

DjLegolas
Copy link
Contributor

@DjLegolas DjLegolas commented Feb 7, 2022

Used the PR from the original repo of the stats plugin.
In addition, fixed some basic bugs.

According to the ticket, still need to add a WebUI... Therefor:
Closes partially: https://dev.deluge-torrent.org/ticket/2812

In addition, while testing, I have found that in case I was on the Stats tab, and maximized the Deluge's window,
the entire GTK would crash in Windows.
When it happened, I got the following error message:

01:01:16.904 [DEBUG   ][deluge.config                     :203 ] Setting key "window_maximized" to: True (of type: <class 'bool'>)
01:01:16.955 [DEBUG   ][deluge.config                     :203 ] Setting key "tabsbar_position" to: 1008 (of type: <class 'int'>)
A s s e r t i o n   f a i l e d :   C A I R O _ R E F E R E N C E _ C O U N T _ H A S _ R E F E R E N C E   ( & s u r f a c e - > r e f _ c o u n t ) ,   f i l e   c a i r o - s u r f a c e . c ,   l i n e   9 5 5 
 
Process finished with exit code -1073740791 (0xC0000409)

Still no fix for this crash.

@cas--
Copy link
Member

cas-- commented Mar 2, 2022

Cairo crash is being tracked in https://dev.deluge-torrent.org/ticket/3339

@cas--
Copy link
Member

cas-- commented Mar 2, 2022

I'm not sure this is ready to be merged.

It should be an optional feature since might be unwanted by existing Stats plugin user and affects the minimum width of the Deluge window.

image

In addition the original TotalTraffic had an option to show just the payload or both total and payload.

The additional core exported method is not needed and can be simplified with maybe_deferred. Below is a quick and dirty working example, don't just copy and paste:

@@ -23,6 +23,7 @@
 import deluge
 from deluge import component
 from deluge.common import fspeed
+from deluge.decorators import maybe_coroutine
 from deluge.plugins.pluginbase import Gtk3PluginBase
 from deluge.ui.client import client
 from deluge.ui.gtk3.torrentdetails import Tab
@@ -280,7 +281,8 @@ def disable(self):
         component.get('StatusBar').remove_item(self.status_item)
         del self.status_item
 
-    def update(self):
+    @maybe_coroutine
+    async def update_status(self):
         def cb_get_status(st):
             def fsize(x, y):
                 return deluge.common.fsize(x[y])
@@ -295,7 +297,13 @@ def format_str(x):
 
             self.status_item.set_text('S: %s|T: %s' % tuple(map(format_str, st)))
 
-        client.stats.get_both_totals().addCallback(cb_get_status)
+        totals = await client.stats.get_totals()
+        session_totals = await client.stats.get_session_totals()
+        cb_get_status((session_totals, totals))
+
+
+    def update(self):
+        self.update_status()

I do also wonder about the number of times get_session_totals is being called but that could be dealt with a later refactor of core.

@DjLegolas DjLegolas force-pushed the update-stats-plugin branch 2 times, most recently from f76ec70 to 6791721 Compare March 4, 2022 16:24
@DjLegolas
Copy link
Contributor Author

I have added the same control in the settings, so one could choose global stats or only payload, and if to show also the total values.
I have also dropped the additional exported method as suggested (but the code is almost identical to yours).

Used the PR from the original repo of the `stats` plugin[1].
In addition, fixed some basic bugs.
The functionality will be the same as the original TotalTraffic.

Closes partially: https://dev.deluge-torrent.org/ticket/2812

[1] ianmartin/Deluge-stats-plugin#5
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.

2 participants