[QT] Add network traffic graph to debug window #2924

pull sje397 wants to merge 1 commits into bitcoin:master from sje397:TrafficGraph changing 13 files +618 −9
  1. sje397 commented at 4:30 PM on August 22, 2013: contributor

    This change adds a Network Traffic tab to the debug window, containing total bytes in/out labels (formatted as KB, MB, GB) and a graph that shows sampled bytes per second in and out. The total time period covered by the graph is adjustable with a slider. snapshot2

  2. luke-jr commented at 5:40 PM on August 22, 2013: member

    Nice. But I worry that people might start getting antsy about bandwidth usage more than they already have...

  3. sje397 commented at 5:45 PM on August 22, 2013: contributor

    @luke-jr That's exactly why I wanted it - because I had moments of antsy-ness about network usage :) I think it's better to be transparent.

  4. BitcoinPullTester commented at 6:21 PM on August 22, 2013: none

    Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/04ffc8f3f4fa4da99567551fae1d6f47c1467edf for binaries and test log.

    This could happen for one of several reasons:

    1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in contrib/test-scripts)
    2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
    3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
    4. The test suite fails on either Linux i386 or Win32
    5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

    If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

    This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  5. petertodd commented at 7:32 PM on August 22, 2013: contributor

    Get rid of the tnow() function and use GetTimeMillis() or GetTimeMicro() instead.

    Also add a "getnetrates" RPC call that dumps this data; it'd be useful for node-monitoring purposes. Specifically it should return the following:

    totalin = nTotalBytesIn totalout = nTotalBytesOut timemillis = (double)GetTimeMillis()/1000

    The time variable lets the callee accurately determine the actual bandwidth rate even if the RPC call has some latency. Also change size_t to uint64 - on 32bit machines size_t is 32-bits which is too small for the total bytes variable.

    Other than those issues I like the idea and will review the code later.

  6. gmaxwell commented at 8:21 PM on August 22, 2013: contributor

    Very very cool. It would be nice if there were some way to break it down— "hey, this one peer is eating all my bandwidth" vs "Bitcoin is eating all my bandwidth, I'll have to stop running Bitcoin" or "Hey, these inbound peers use a lot of bandwidth".

  7. sje397 commented at 11:47 PM on August 22, 2013: contributor

    @petertodd I was sure there'd be a better time func in there somewhere - cheers. @gmaxwell Thanks. Yeah, would be good to take it further.

  8. sje397 commented at 1:20 AM on August 23, 2013: contributor

    @petertodd The problem with those time functions is that they use wall-clock time. Is it possible to use boost's 'steady clock' or we need to support versions of boost that are too old for that?

  9. petertodd commented at 6:04 AM on August 23, 2013: contributor

    @sje397 I'm not familiar enough with boost to answer that.

    Would someone else please answer his question.

  10. gavinandresen commented at 8:20 AM on August 23, 2013: contributor

    @sje397 : Looks like we gitian-build against boost 1.40. Updating the version of Ubuntu we use for deterministic building and updating dependencies is on my want-to-happen-for-0.9 list...

  11. laanwj commented at 8:40 AM on August 23, 2013: member

    Looks very nice.

    If you intend to keep tnow (I can see why, as you need monotonic time here, and AFAIK neither Qt nor Boost offers such a function) please move it to either util.cpp or guiutil.cpp.

  12. in src/qt/rpcconsole.cpp:None in 04ffc8f3f4 outdated 
     442 | +void RPCConsole::on_sldGraphRange_valueChanged(int value)
 443 | +{
 444 | +    const int multiplier = 15; // each position on the slider represents 15 min
 445 | +    int mins = value * multiplier;
 446 | +    ui->trafficGraph->setGraphRangeMins(mins);
 447 | +    ui->lblGraphRange->setText(QString("%1%2").arg(mins < 60 ? mins : mins / 60.0f).arg(mins < 60 ? "m" : "h"));
    Diapolo commented at 10:00 AM on August 23, 2013:

    Used units should be translatable IMHO :). Or even better use system locale or selected language?

  13. in src/netrates.cpp:None in 04ffc8f3f4 outdated 
      14 | +{
  15 | +    float tnow()
  16 | +    {
  17 | +        struct timespec ts;
  18 | +
  19 | +        #ifdef __MACH__ // OS X does not have clock_gettime, use clock_get_time
    sipa commented at 3:00 PM on August 25, 2013:

    Coding style nit: can you make macro statements unindented?

  14. in src/bitcoinrpc.cpp:None in 5fdd9b562c outdated 
     199 | @@ -200,6 +200,7 @@ Value stop(const Array& params, bool fHelp)
 200 |      { "getpeerinfo",            &getpeerinfo,            true,      false },
 201 |      { "addnode",                &addnode,                true,      true },
 202 |      { "getaddednodeinfo",       &getaddednodeinfo,       true,      true },
 203 | +	{ "getnetrates",			&getnetrates,			 true,		true },
    Diapolo commented at 12:12 PM on August 26, 2013:

    Indentation

  15. in src/netrates.cpp:None in 5fdd9b562c outdated 
       7 | +{
   8 | +
   9 | +
  10 | +CCriticalSection cs_rateDataIn, cs_rateDataOut;
  11 | +
  12 | +uint64 intime = GetTimeMonotonicMillis(), outtime = intime;
    Diapolo commented at 12:13 PM on August 26, 2013:

    Can you please use a single line for every variable, I'm sure @laanwj told me this sometime ago ^^.

  16. in src/netrates.cpp:None in 5fdd9b562c outdated 
      44 | +    }
  45 | +
  46 | +    float GetSampleBytesPerSecondIn()
  47 | +    {
  48 | +        LOCK(cs_rateDataIn);
  49 | +		uint64 now = GetTimeMonotonicMillis();
    Diapolo commented at 12:14 PM on August 26, 2013:

    Indentation, same below ^^.

  17. in src/qt/rpcconsole.cpp:None in 5fdd9b562c outdated 
     208 | @@ -206,6 +209,11 @@ void RPCExecutor::request(const QString &command)
 209 |      startExecutor();
 210 |  
 211 |      clear();
 212 | +
 213 | +    QTimer *timer = new QTimer(this);
 214 | +    timer->setInterval(2000);
    Diapolo commented at 12:17 PM on August 26, 2013:

    Perhaps make this a constant, see top of rpcconsole code below the Todos.

  18. in src/bitcoinrpc.cpp:None in 6bf05cab94 outdated 
     199 | @@ -200,6 +200,7 @@ Value stop(const Array& params, bool fHelp)
 200 |      { "getpeerinfo",            &getpeerinfo,            true,      false },
 201 |      { "addnode",                &addnode,                true,      true },
 202 |      { "getaddednodeinfo",       &getaddednodeinfo,       true,      true },
 203 | +    { "getnetrates",			&getnetrates,			 true,		true },
    Diapolo commented at 12:18 PM on August 26, 2013:

    Still indentation ;).

  19. in src/qt/trafficgraphwidget.cpp:None in f2cc741058 outdated 
       0 | @@ -0,0 +1,138 @@
   1 | +#include "trafficgraphwidget.h"
   2 | +#include "../netrates.h"
   3 | +#include "ui_interface.h"
   4 | +
   5 | +#include <QTimer>
    Diapolo commented at 12:29 PM on August 26, 2013:

    This already comes from the header, should be safe to remove.

  20. in src/rpcnet.cpp:None in f2cc741058 outdated 
     203 | @@ -202,3 +204,17 @@ Value getaddednodeinfo(const Array& params, bool fHelp)
 204 |      return ret;
 205 |  }
 206 |  
 207 | +Value getnetrates(const Array& params, bool fHelp)
 208 | +{
 209 | +	if (fHelp || params.size() > 0)
 210 | +		throw runtime_error(
    Diapolo commented at 12:30 PM on August 26, 2013:

    Some indentation problems here...

  21. sje397 commented at 12:38 PM on August 26, 2013: contributor

    Thanks Diapolo.

  22. in src/qt/trafficgraphwidget.h:None in 999a7eecd3 outdated 
      24 | +    void paintPath(QPainterPath &path, QQueue<float> &samples);
  25 | +
  26 | +    QTimer timer;
  27 | +    float fMax;
  28 | +    int nMins;
  29 | +    QQueue<float> vSamplesIn;
    Diapolo commented at 12:56 PM on August 26, 2013:

    These 2 don't need an initialisation in the constructor?

    sje397 commented at 4:10 PM on August 26, 2013:

    Done, ta.

  23. in src/qt/trafficgraphwidget.h:None in 999a7eecd3 outdated 
      21 | +    void setGraphRangeMins(int mins);
  22 | +
  23 | +private:
  24 | +    void paintPath(QPainterPath &path, QQueue<float> &samples);
  25 | +
  26 | +    QTimer timer;
    Diapolo commented at 1:01 PM on August 26, 2013:

    Should this better be a pointer? Initialized to 0 via constructor and something like

    <pre> timer = new QTimer(this); connect(timer, SIGNAL(timeout()), SLOT(updateRates())); </pre>

    I looked through current Qt code and it seems we use a pointer everywhere else. @laanwj Can you comment what is preferred here?

  24. in src/rpcnet.cpp:None in 999a7eecd3 outdated 
     210 | +        throw runtime_error(
 211 | +            "getnetrates\n"
 212 | +            "Returns information about network traffic, including bytes in, bytes out, and current (monotonic) time.");
 213 | +
 214 | +    Object obj;
 215 | +    obj.push_back(Pair("totalin", NetRates::GetTotalIn()));
    Diapolo commented at 1:03 PM on August 26, 2013:

    getpeerinfo uses bytessent and bytesrecv @sipa should we use similar or compatible namings for these?

  25. sje397 commented at 2:52 PM on August 26, 2013: contributor

    I think steady_clock is available in boost >= 1.47, and QElapsedTimer supports monotonic time in Qt >= 4.7 - both unfortunately recent.

  26. sje397 commented at 3:23 PM on August 26, 2013: contributor

    Maybe it would be better to make the total byte counts static members of the CNode class, and do the timing on the gui side.

  27. sje397 commented at 4:08 PM on August 26, 2013: contributor

    Sorry for the late refactor, but I think this cleans things up significantly.

  28. in src/qt/rpcconsole.cpp:None in b030ad141b outdated 
       3 | @@ -4,8 +4,10 @@
   4 |  #include "clientmodel.h"
   5 |  #include "bitcoinrpc.h"
   6 |  #include "guiutil.h"
   7 | +#include "../net.h"
    Diapolo commented at 6:11 AM on August 27, 2013:

    It is now cleaner, as no new header and cpp is needed, but now you access stuff in the core directly, which should/could be circumvented, if you would add a getter in clientmodel (@laanwj What do you think?). You don't need to change this yet, I would wait for further comments :).

    sje397 commented at 7:52 AM on August 27, 2013:

    I was hoping to move toward gmaxwell's suggestion of breaking things down, so I thought it useful to be getting at the CNode data (since a lot is already there)...I'll look into the clientmodel - I know nothing about it atm.

    sje397 commented at 9:43 AM on August 27, 2013:

    I think you're right Diapolo - and that should allow me to remove the timer I added in this file too, and base updates on the model updates.

  29. in src/rpcnet.cpp:None in b030ad141b outdated 
     209 | +        throw runtime_error(
 210 | +            "getnetrates\n"
 211 | +            "Returns information about network traffic, including bytes in, bytes out, and current (monotonic) time.");
 212 | +
 213 | +    Object obj;
 214 | +    obj.push_back(Pair("totalin", CNode::GetTotalBytesIn()));
    Diapolo commented at 6:16 AM on August 27, 2013:

    I still suggest something like totalbytesin or bytesintotal to have the unit in.

    sje397 commented at 7:24 AM on August 27, 2013:

    Yep - 'getnetrates' isn't entirely appropriate either, since it's just totals.

  30. in src/qt/forms/rpcconsole.ui:None in 829d19be7d outdated 
     696 | +               <enum>Qt::Vertical</enum>
 697 | +              </property>
 698 | +              <property name="sizeHint" stdset="0">
 699 | +               <size>
 700 | +                <width>20</width>
 701 | +                <height>407</height>
    Diapolo commented at 1:32 PM on August 27, 2013:

    Perhaps this value prevents you from shrinking it?

    sje397 commented at 1:35 PM on August 27, 2013:

    I tried setting that to 20, but it didn't help. It looks to me like the lines of text in the first tab come pretty close to that height anyway (on my macbook)...I can have another shot on my other machine.

    Diapolo commented at 1:39 PM on August 27, 2013:

    What if you edit the size in the .ui file directly?

    sje397 commented at 1:57 PM on August 27, 2013:

    It works fine, but then changes to 533 if I open it. That could be annoying. I'll revert it anyway.

    Diapolo commented at 2:00 PM on August 27, 2013:

    Are you using Qt Creator IDE / Qt Designer for editting the .ui file? Which version are you on?

    sje397 commented at 2:04 PM on August 27, 2013:

    Qt Creator IDE 2.4.1, on OS X 10.8.4.

    Diapolo commented at 2:12 PM on August 27, 2013:

    I'm using 2.8.0 currently, any reson for you not to upgrade?

    sje397 commented at 2:17 PM on August 27, 2013:

    Will do, when I get a chance. I'm not sure what version I have on my Linux machine.

  31. in src/qt/trafficgraphwidget.h:None in 829d19be7d outdated 
       0 | @@ -0,0 +1,39 @@
   1 | +#ifndef TRAFFICGRAPHWIDGET_H
   2 | +#define TRAFFICGRAPHWIDGET_H
   3 | +
   4 | +#include <QWidget>
   5 | +#include <QQueue>
   6 | +
   7 | +class QPaintEvent;
    Diapolo commented at 1:37 PM on August 27, 2013:

    The Qt classes should be guarded by: QT_BEGIN_NAMESPACE QT_END_NAMESPACE

    And ClientModel should be in front of that block then.

    sje397 commented at 1:46 PM on August 27, 2013:

    Done.

  32. Diapolo commented at 1:39 PM on August 27, 2013: none

    Qt code looks much better now, good job and thanks for listening :).

  33. sje397 commented at 1:50 PM on August 27, 2013: contributor

    @Diapolo No probs. Thanks for your time and patience.

  34. Diapolo commented at 1:56 PM on August 27, 2013: none

    I would say some core-devs should comment on the core changes and RPC stuff now and @laanwj should review the Qt code. If I find the time I'll try to compile your patch also if you consider it test-ready :).

    One feature request, maybe you could add a clear button?

  35. sje397 commented at 2:02 PM on August 27, 2013: contributor

    Yeah sure, please try it out. It does get boring pretty fast :) Along the way I added a couple of coloured lines next to the 'in' and 'out' labels to explain the graph a bit.

  36. Diapolo commented at 2:26 PM on August 27, 2013: none

    Alright, compiles fine and is working. A few comments:

    1. I think it's better readable when you place a space between the values and the unit on the right (dunno if this is a locale thing, I'm just offering just my personal impression).
    2. When changing the slider the graphs are reset, guess this is normal?
    3. When changing the slider it can give numers like 13.75h any way to make this depend in users locales or perhaps split hours and minutes at least?
    4. Can't say why, but 15 minutes resolution sounds too large, graphics look not sharp or clear, I'm missing my wow-factor when looking at it ^^.
    5. I wonder why the graphs are only drawn if there is traffic, perhaps I'm missunderstanding something, but if I select 15mins the graph should be pretty much moving steady from right to left?
  37. sje397 commented at 2:50 PM on August 27, 2013: contributor

    Thanks again Diapolo. Yep, changing the slider clears the graph because I didn't want to get complicated with resampling or storing timestamps etc.

    I moved the units into the translated string - that should make it more adjustable, e.g. for locales that might put the unit before the number etc. I put the spaces in, and dropped the resolution down to 5 mins.

  38. sje397 commented at 2:53 PM on August 27, 2013: contributor

    I'm not sure what's up with your #5 - I get the graph moving steadily, even with no network connection.

  39. Diapolo commented at 10:54 AM on August 28, 2013: none

    You were right, it is moving, sorry :). Thanks for the other changes!

    One nit, I saw you connected the clicked() signal to clear() via .ui file, whereas I would prefer: connect(ui->btnClearTrafficGraph, SIGNAL(clicked()), ui->trafficGraph, SLOT(clear())); in the RPCConsole constructor.

    I know it's THAT easy to connect signals and slots via Qt Designer, but most of the .ui stuff is setup in the code and not in the .ui files. My personal oppinion is that such connections get lost, when browsing code and forgetting about .ui files.

  40. in src/qt/forms/rpcconsole.ui:None in d096dd2907 outdated 
     590 | +                  <width>50</width>
 591 | +                  <height>0</height>
 592 | +                 </size>
 593 | +                </property>
 594 | +                <property name="text">
 595 | +                 <string>0b</string>
    Diapolo commented at 10:57 AM on August 28, 2013:

    Can you please remove the string here. If you leave it we will see 0b on Transifex to get translated ;).

  41. in src/qt/forms/rpcconsole.ui:None in d096dd2907 outdated 
     673 | +                  <width>50</width>
 674 | +                  <height>0</height>
 675 | +                 </size>
 676 | +                </property>
 677 | +                <property name="text">
 678 | +                 <string>0b</string>
    Diapolo commented at 10:57 AM on August 28, 2013:

    Can you please remove the string here. If you leave it we will see 0b on Transifex to get translated ;).

  42. in src/qt/forms/rpcconsole.ui:None in d096dd2907 outdated 
     490 | +               <width>100</width>
 491 | +               <height>0</height>
 492 | +              </size>
 493 | +             </property>
 494 | +             <property name="text">
 495 | +              <string>30m</string>
    Diapolo commented at 11:17 AM on August 28, 2013:

    Same here, as it also would result in a nonsense translation string.

  43. laanwj commented at 7:34 PM on August 28, 2013: member

    Getting build errors here:

    projects/bitcoin/src/rpcnet.cpp:214: error: conversion from 'uint64 {aka long long unsigned int}' to 'const Value_type {aka const json_spirit::Value_impl<json_spirit::Config_vector<std::basic_string<char> > >}' is ambiguous
projects/bitcoin/src/rpcnet.cpp:215: error: conversion from 'uint64 {aka long long unsigned int}' to 'const Value_type {aka const json_spirit::Value_impl<json_spirit::Config_vector<std::basic_string<char> > >}' iambiguous
projects/bitcoin/src/rpcnet.cpp:216: error: conversion from 'int64 {aka long long int}' to 'const Value_type {aka const json_spirit::Value_impl<json_spirit::Config_vector<std::basic_string<char> > >}' is ambiguous
  44. sje397 commented at 4:38 AM on August 29, 2013: contributor

    @laanwj Will look tonight. Odd that it passed the auto tests, and I didn't see it on osx or linux...will have to get my win build working I guess.

  45. Diapolo commented at 3:47 PM on August 29, 2013: none

    I would LOVE to be able to get graphs like Tor/Vidala for Windows allows (solid with mild transparency): tor

  46. Diapolo commented at 4:39 PM on August 29, 2013: none

    After removing the default string for the time window, you need to make sure the on_sldGraphRange_valueChanged code is called during init, because if not the initial display of selected slider value is missing.

  47. sje397 commented at 4:14 PM on September 5, 2013: contributor

    @laanwj Please try again - I think I've resolved those errors. @Diapolo I added the alpha background and fixed the issue with the slider value display.

  48. laanwj commented at 7:41 AM on September 7, 2013: member

    Yep, builds now,thanks!

    Edit: this is wicked cool, definitely want this in

  49. in src/qt/forms/rpcconsole.ui:None in 7b38225203 outdated 
     489 | +              <size>
 490 | +               <width>100</width>
 491 | +               <height>0</height>
 492 | +              </size>
 493 | +             </property>
 494 | +             <property name="text">
    Diapolo commented at 2:59 PM on September 13, 2013:

    This and the 2 following lines can and should be removed.

    Diapolo commented at 1:05 PM on September 19, 2013:

    Alert ^^, you forgot this one :).

  50. in src/qt/forms/rpcconsole.ui:None in 7b38225203 outdated 
     589 | +                 <size>
 590 | +                  <width>50</width>
 591 | +                  <height>0</height>
 592 | +                 </size>
 593 | +                </property>
 594 | +                <property name="text">
    Diapolo commented at 3:00 PM on September 13, 2013:

    This and the 2 following lines can and should be removed.

  51. in src/qt/forms/rpcconsole.ui:None in 7b38225203 outdated 
     672 | +                 <size>
 673 | +                  <width>50</width>
 674 | +                  <height>0</height>
 675 | +                 </size>
 676 | +                </property>
 677 | +                <property name="text">
    Diapolo commented at 3:02 PM on September 13, 2013:

    This and the 2 following lines can and should be removed.

  52. in src/qt/trafficgraphwidget.h:None in 7b38225203 outdated 
      16 | +    Q_OBJECT
  17 | +public:
  18 | +    explicit TrafficGraphWidget(QWidget *parent = 0);
  19 | +    void setClientModel(ClientModel *model);
  20 | +    int getGraphRangeMins() const;
  21 | +protected:
    Diapolo commented at 3:03 PM on September 13, 2013:

    Nit: Can you add a new-line before this and remove signals: as there currently is none :).

  53. Diapolo commented at 3:05 PM on September 13, 2013: none

    Looks much better with the alpha background, if you now rebase (because of that autotools stuff) and fix my minor nits, you have my ACK.

  54. sje397 commented at 11:58 AM on September 19, 2013: contributor

    Done. Thanks again, @Diapolo.... Now to figure out how to build the thing without a pro file :)

  55. Diapolo commented at 12:11 PM on September 19, 2013: none

    You don't need to build it for yourself without the .pro file (I also don't do this) you AFAIK just need to make sure you add the new .cpp and .h files in src/qt/Makefile.am to make pulltester happy and have this mergeable.

  56. sje397 commented at 12:17 PM on September 19, 2013: contributor

    Grr. Autotools is complaining about missing libdb_cxx headers - looking for version 48 and I have version 46 and version 53...

    I'll fix that makefile. Thanks.

  57. sipa commented at 12:21 PM on September 19, 2013: member

    This is intentional. Use --with-incompatible-bdb if you want to use a different version than 4.8.

  58. sje397 commented at 12:34 PM on September 19, 2013: contributor

    Thanks @sipa - no worries installing db48... I just thought it was going to be a 5 min fixup, that's all... now it can't find Qt... I'll figure it out.

  59. in src/qt/rpcconsole.cpp:None in de32065d2c outdated 
     467 | +        int hours = mins / 60;
 468 | +        int minsLeft = mins % 60;
 469 | +        if(minsLeft == 0) {
 470 | +            ui->lblGraphRange->setText(QString(tr("%1 h")).arg(hours));
 471 | +        } else {
 472 | +            ui->lblGraphRange->setText(QString(tr("%1 h %3 m")).arg(hours).arg(minsLeft));
    Diapolo commented at 1:07 PM on September 19, 2013:

    I wonder why this is %3 here? Shouldn't that be %2?

    sje397 commented at 1:10 PM on September 19, 2013:

    Absolutely :)

  60. sje397 commented at 1:19 PM on September 19, 2013: contributor

    Got it almost building, and modified src/qt/Makefile.am to moc and compile the new widget...

    Any ideas about this build error?

    OBJCXX libbitcoinqt_a-macdockiconhandler.o In file included from /System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:123:0, from /System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12, from /opt/local/include/QtGui/qmacdefines_mac.h:113, from /opt/local/include/QtGui/qwindowdefs.h:99, from /opt/local/include/QtGui/qwidget.h:46, from /opt/local/include/QtGui/qmainwindow.h:45, from /opt/local/include/QtGui/QMainWindow:1, from macdockiconhandler.h:5, from macdockiconhandler.mm:1: /System/Library/Frameworks/Foundation.framework/Headers/NSTask.h:69:24: error: expected unqualified-id before '^' token /System/Library/Frameworks/Foundation.framework/Headers/NSTask.h:69:24: error: expected ')' before '^' token

  61. sje397 commented at 2:02 PM on September 19, 2013: contributor

    Nevermind, it seems setting macports to use the llvm-gcc compiler works much better :)

  62. Diapolo commented at 12:19 PM on September 20, 2013: none

    ACK to current UI and Qt changes. I would love to see some core devs comment on the naming of getnettotals and it's fields (totalbytesin and totalbytesout) or even get ACKs now!

    I'm happy to see this evolving even further :).

  63. Diapolo commented at 11:28 AM on September 25, 2013: none

    @laanwj ping

  64. in src/rpcnet.cpp:None in f8cc0779bb outdated 
     211 | +            "and current time.");
 212 | +
 213 | +    Object obj;
 214 | +    obj.push_back(Pair("totalbytesin", static_cast< boost::uint64_t>(CNode::GetTotalBytesIn())));
 215 | +    obj.push_back(Pair("totalbytesout", static_cast<boost::uint64_t>(CNode::GetTotalBytesOut())));
 216 | +    obj.push_back(Pair("timemillis", static_cast<boost::int64_t>(GetTimeMillis())));
    sipa commented at 2:19 PM on September 29, 2013:

    What is this useful for?

    sipa commented at 2:20 PM on September 29, 2013:

    Ah, calculating average speeds over polling intervals by external applications I gues. Ok.

  65. laanwj commented at 7:39 AM on October 6, 2013: member

    @diapolo If this was a GUI-only change I'd have merged it quite some time ago.

    Maybe split adding the RPC call into a different commit so it gets more review by the other devs?

  66. Diapolo commented at 12:05 PM on October 6, 2013: none

    @laanwj Well even if @sje397 leaves out the rpc-call, there are still changes to the core (at least adding some vars for counting the traffic), which needs core-devs ACK :-/. I'm fine with your split idea, if this helps speed things up...

  67. in src/qt/Makefile.am:None in f8cc0779bb outdated 
      47 | @@ -48,7 +48,7 @@ QT_MOC_CPP = moc_aboutdialog.cpp moc_addressbookpage.cpp \
  48 |    moc_optionsmodel.cpp moc_overviewpage.cpp moc_paymentserver.cpp \
  49 |    moc_qrcodedialog.cpp moc_qvalidatedlineedit.cpp moc_qvaluecombobox.cpp \
  50 |    moc_rpcconsole.cpp moc_sendcoinsdialog.cpp moc_sendcoinsentry.cpp \
  51 | -  moc_signverifymessagedialog.cpp moc_splashscreen.cpp moc_transactiondesc.cpp \
  52 | +  moc_signverifymessagedialog.cpp moc_splashscreen.cpp moc_trafficgraphwidget.cpp moc_transactiondesc.cpp \
    Diapolo commented at 12:06 PM on October 6, 2013:

    I'm thinking about a one-liner in the qt Makefile.am for every file we need to add... I think this gets cluttered very quickly. @laanwj What do you think? Not related to this pull but general direction.

    laanwj commented at 3:55 PM on October 7, 2013:

    I personally always use one-per-line as well, as that results in better readable diffs, having said that, I don't care enough about it to change it :)

  68. laanwj commented at 4:04 PM on October 7, 2013: member

    Yes that's true, it's not possible to entirely split it into core + GUI parts. So don't do the split.

    Let's just get at least one more ACK on the core changes. @gmaxwell what do you think about the network changes? Your per-peer comment is not addressed yet but that could be done in a later pull, I think we should merge this if there are no blocking issues.

  69. in src/net.cpp:None in f8cc0779bb outdated 
     425 | @@ -426,8 +426,10 @@ void AddressCurrentlyConnected(const CService& addr)
 426 |  
 427 |  
 428 |  
 429 | -
 430 | -
 431 | +uint64 CNode::nTotalBytesIn = 0;
 432 | +uint64 CNode::nTotalBytesOut = 0;
 433 | +CCriticalSection CNode::cs_trafficStatsIn;
    sipa commented at 8:54 PM on October 13, 2013:

    Can you give the critical sections the same name as the variable they're protecting (given that it's just a single variable)?

  70. in src/rpcnet.cpp:None in f8cc0779bb outdated 
     209 | +            "getnettotals\n"
 210 | +            "Returns information about network traffic, including bytes in, bytes out,\n"
 211 | +            "and current time.");
 212 | +
 213 | +    Object obj;
 214 | +    obj.push_back(Pair("totalbytesin", static_cast< boost::uint64_t>(CNode::GetTotalBytesIn())));
    sipa commented at 8:57 PM on October 13, 2013:

    Maybe call these 'totalbytessent' and 'totalbytesrecv', for consistency with the variables in getpeerinfo?

  71. sipa commented at 8:57 PM on October 13, 2013: member

    ACK on the changes to core (didn't look at the GUI), apart from a few nits above.

    Needs rebase, though.

  72. Add network traffic graph ce14345a89
  73. BitcoinPullTester commented at 8:26 AM on October 14, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/ce14345a89dfa05992f8d2c7c9fe36315d4a67e6 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  74. laanwj referenced this in commit f0c25cf6ec on Oct 15, 2013
  75. laanwj merged this on Oct 15, 2013
  76. laanwj closed this on Oct 15, 2013
  77. sje397 commented at 10:49 AM on October 15, 2013: contributor

    Thanks guys!

  78. sje397 deleted the branch on Oct 15, 2013
  79. laanwj commented at 11:02 AM on October 15, 2013: member

    Thanks to you for making this

  80. rebroad commented at 12:02 AM on February 18, 2014: contributor

    Thanks @sje397 for this! Do you have a donation address?

  81. sje397 commented at 1:19 PM on February 19, 2014: contributor

    198FVLAZ7ZMRRFtZ17fXsDWJnHdP5bxiNn if you must, @rebroad , but it was my pleasure.

  82. DrahtBot locked this on Sep 8, 2021
Contributors
sje397luke-jrBitcoinPullTesterpetertoddgmaxwellgavinandresenlaanwjDiapolosiparebroad
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-29 03:16 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me