-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Display external address #20118
base: master
Are you sure you want to change the base?
Display external address #20118
Changes from all commits
a398dc4
2323e9a
c6cd439
d5888aa
4e07c0a
91d075a
37a9eda
7ae5783
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,6 +86,15 @@ StatusBar::StatusBar(QWidget *parent) | |
m_upSpeedLbl->setStyleSheet(u"text-align:left;"_s); | ||
m_upSpeedLbl->setMinimumWidth(200); | ||
|
||
m_lastExternalAddressesLbl = new QPushButton(this); | ||
m_lastExternalAddressesLbl->setText(tr("External Address(es): Detecting")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about changing "Detecting" to "N/A"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of course, the IP address will still be longer. But in any case, all we can say about this is that the information about external address is currently "not available". IMO, "Detecting" looks speculative. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know how that would differentiate between "Not available...yet" and other states such as failed, but I do agree about 'Detecting' sounding speculative. Perhaps more information should be communicated to the frontend from the means that qBittorrent uses to detect. "Detecting/Detection failed" etc? As for how much space is used...at this rate it's looking like you want just external addresses printed and nothing else which, quite frankly, leaves me baffled as to what the information would be. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then again, some people use torrent clients on closed networks (so local IP address(es)), so maybe it can make sense. |
||
m_lastExternalAddressesLbl->setIcon(UIThemeManager::instance()->getIcon(u"help-about"_s)); | ||
m_lastExternalAddressesLbl->setFlat(true); | ||
m_lastExternalAddressesLbl->setFocusPolicy(Qt::NoFocus); | ||
m_lastExternalAddressesLbl->setCursor(Qt::PointingHandCursor); | ||
m_lastExternalAddressesLbl->setSizePolicy(QSizePolicy::Maximum, QSizePolicy::Preferred); | ||
connect(m_lastExternalAddressesLbl, &QAbstractButton::clicked, this, &StatusBar::showExternalAddressesButtonClicked); | ||
|
||
m_DHTLbl = new QLabel(tr("DHT: %1 nodes").arg(0), this); | ||
m_DHTLbl->setSizePolicy(QSizePolicy::Maximum, QSizePolicy::Preferred); | ||
|
||
|
@@ -129,14 +138,21 @@ StatusBar::StatusBar(QWidget *parent) | |
#ifndef Q_OS_MACOS | ||
statusSep4->setFrameShadow(QFrame::Raised); | ||
#endif | ||
layout->addWidget(m_DHTLbl); | ||
QFrame *statusSep5 = new QFrame(this); | ||
statusSep5->setFrameStyle(QFrame::VLine); | ||
#ifndef Q_OS_MACOS | ||
statusSep5->setFrameShadow(QFrame::Raised); | ||
#endif | ||
layout->addWidget(m_lastExternalAddressesLbl); | ||
layout->addWidget(statusSep1); | ||
layout->addWidget(m_connecStatusLblIcon); | ||
layout->addWidget(m_DHTLbl); | ||
layout->addWidget(statusSep2); | ||
layout->addWidget(m_connecStatusLblIcon); | ||
layout->addWidget(statusSep3); | ||
layout->addWidget(m_altSpeedsBtn); | ||
layout->addWidget(statusSep4); | ||
layout->addWidget(m_dlSpeedLbl); | ||
layout->addWidget(statusSep3); | ||
layout->addWidget(statusSep5); | ||
layout->addWidget(m_upSpeedLbl); | ||
|
||
addPermanentWidget(container); | ||
|
@@ -212,6 +228,31 @@ void StatusBar::updateDHTNodesNumber() | |
} | ||
} | ||
|
||
void StatusBar::updateExternalAddressesLabel() | ||
{ | ||
const QString lastExternalIPv4Address = BitTorrent::Session::instance()->getLastExternalIPv4Address(); | ||
const QString lastExternalIPv6Address = BitTorrent::Session::instance()->getLastExternalIPv6Address(); | ||
QString addressText = tr("External Address(es): Detecting"); | ||
|
||
if (!lastExternalIPv4Address.isEmpty() || !lastExternalIPv6Address.isEmpty()) | ||
{ | ||
if (!lastExternalIPv4Address.isEmpty() && !lastExternalIPv6Address.isEmpty()) | ||
{ | ||
addressText = tr("External Addresses: %1, %2") | ||
.arg(BitTorrent::Session::instance()->getLastExternalIPv4Address()) | ||
.arg(BitTorrent::Session::instance()->getLastExternalIPv6Address()); | ||
} | ||
else | ||
{ | ||
addressText = tr("External Address: %1%2") | ||
.arg(BitTorrent::Session::instance()->getLastExternalIPv4Address()) | ||
.arg(BitTorrent::Session::instance()->getLastExternalIPv6Address()); | ||
} | ||
} | ||
|
||
m_lastExternalAddressesLbl->setText(addressText); | ||
} | ||
|
||
void StatusBar::updateSpeedLabels() | ||
{ | ||
const BitTorrent::SessionStatus &sessionStatus = BitTorrent::Session::instance()->status(); | ||
|
@@ -235,6 +276,7 @@ void StatusBar::refresh() | |
{ | ||
updateConnectionStatus(); | ||
updateDHTNodesNumber(); | ||
updateExternalAddressesLabel(); | ||
updateSpeedLabels(); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,6 +82,8 @@ namespace | |
|
||
// TransferInfo keys | ||
const QString KEY_TRANSFER_CONNECTION_STATUS = u"connection_status"_s; | ||
const QString KEY_TRANSFER_LAST_EXTERNAL_ADDRESS_V6 = u"last_external_address_v6"_s; | ||
const QString KEY_TRANSFER_LAST_EXTERNAL_ADDRESS_V4 = u"last_external_address_v4"_s; | ||
Comment on lines
+85
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrong order, keys are sorted alphabetically here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ... I didn't see anything that required it. |
||
const QString KEY_TRANSFER_DHT_NODES = u"dht_nodes"_s; | ||
const QString KEY_TRANSFER_DLDATA = u"dl_info_data"_s; | ||
const QString KEY_TRANSFER_DLRATELIMIT = u"dl_rate_limit"_s; | ||
|
@@ -161,6 +163,8 @@ namespace | |
map[KEY_TRANSFER_AVERAGE_TIME_QUEUE] = cacheStatus.averageJobTime; | ||
map[KEY_TRANSFER_TOTAL_QUEUED_SIZE] = cacheStatus.queuedBytes; | ||
|
||
map[KEY_TRANSFER_LAST_EXTERNAL_ADDRESS_V6] = session->getLastExternalIPv6Address(); | ||
map[KEY_TRANSFER_LAST_EXTERNAL_ADDRESS_V4] = session->getLastExternalIPv4Address(); | ||
map[KEY_TRANSFER_DHT_NODES] = sessionStatus.dhtNodes; | ||
map[KEY_TRANSFER_CONNECTION_STATUS] = session->isListening() | ||
? (sessionStatus.hasIncomingConnections ? u"connected"_s : u"firewalled"_s) | ||
|
@@ -445,6 +449,8 @@ void SyncController::updateFreeDiskSpace(const qint64 freeDiskSpace) | |
// - "total_size": Size including unwanted data | ||
// Server state map may contain the following keys: | ||
// - "connection_status": connection status | ||
// - "last_external_address_v6": last external address v6 | ||
// - "last_external_address_v4": last external address v4 | ||
// - "dht_nodes": DHT nodes count | ||
// - "dl_info_data": bytes downloaded | ||
// - "dl_info_speed": download speed | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -843,6 +843,21 @@ window.addEvent('load', function() { | |||||||||||||
else | ||||||||||||||
document.title = ("qBittorrent " + qbtVersion() + " QBT_TR(Web UI)QBT_TR[CONTEXT=OptionsDialog]"); | ||||||||||||||
$('freeSpaceOnDisk').set('html', 'QBT_TR(Free space: %1)QBT_TR[CONTEXT=HttpServer]'.replace("%1", window.qBittorrent.Misc.friendlyUnit(serverState.free_space_on_disk))); | ||||||||||||||
|
||||||||||||||
let last_external_address_v4 = serverState.last_external_address_v4; | ||||||||||||||
let last_external_address_v6 = serverState.last_external_address_v6; | ||||||||||||||
Comment on lines
+847
to
+848
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I followed the previous conventions used by qBittorrent for other variables used similarly. |
||||||||||||||
let last_external_addresses = 'QBT_TR(External Address(es): Detecting)QBT_TR[CONTEXT=HttpServer]'; | ||||||||||||||
|
||||||||||||||
if (last_external_address_v4 !== "" || last_external_address_v6 !== "") | ||||||||||||||
{ | ||||||||||||||
if (last_external_address_v4 !== "" && last_external_address_v6 !== "") | ||||||||||||||
Comment on lines
+851
to
+853
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
last_external_addresses = 'QBT_TR(External Addresses: %1, %2)QBT_TR[CONTEXT=HttpServer]'; | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although unlikely, I would still suggest using other character for replacement specifier. Maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't think any device was allowed to start with a number, so I didn't think there any chance of running into %[0-9] with %[0-9] being some interface. I could be wrong but I thought no device could start with [0-9]. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately, according to the linked article, Windows will use %[0-9] format:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the WebUI, I believe it should work to do this in reverse-order. Ex:
Similarly I think the GUI should do the same (reverse order) and always specifically replace the first occurrence only. Edit: I'm away from rig for the day. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I forgot to ask, does qB supply those interfacings or does qB just supply the IP address? If it's IP address then we don't need to account for which interface (unless interface is added to IP addresses). My only test case for this info has been Linux. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I must've been thinking about another client then. Still, there isn't any way for a peer or tracker to know which interface the client is using, so the entire point seems moot. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't know much about IPv6. What exactly are you talking about when you mention the interface? About the zone identifier, e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Somewhere on this long thread someone mentioned something about not using Edit: Too long a thread, unable to search and follow it anymore hardly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
IIRC, Zone ID is "locally significant" (enables us to define out which interface we want to send some traffic) and there is no point to send it to remote host as part if its "external address". But we still can't be sure that it isn't sent in such a way (with Zone ID) so the question is how is it handled by libtorrent, i.e. can it provide we "external address" that mistakenly contains such Zone ID. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Someone familiar with libtorrent's API should weigh in. I resorted to a VM script, so I'm fine and qBittorrent can do with this PR as they want. |
||||||||||||||
else | ||||||||||||||
last_external_addresses = 'QBT_TR(External Address: %1%2)QBT_TR[CONTEXT=HttpServer]'; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
$('freeSpaceOnDisk').set('html', 'QBT_TR(Free space: %1)QBT_TR[CONTEXT=HttpServer]'.replace("%1", window.qBittorrent.Misc.friendlyUnit(serverState.free_space_on_disk))); | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. copy paste error? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. -_-; Too tired to address right now, will do later. |
||||||||||||||
$('externalAddresses').set('html', last_external_addresses.replace("%1", last_external_address_v4).replace("%2", last_external_address_v6)); | ||||||||||||||
$('DHTNodes').set('html', 'QBT_TR(DHT: %1 nodes)QBT_TR[CONTEXT=StatusBar]'.replace("%1", serverState.dht_nodes)); | ||||||||||||||
|
||||||||||||||
// Statistics dialog | ||||||||||||||
|
@@ -1024,6 +1039,17 @@ window.addEvent('load', function() { | |||||||||||||
updateTabDisplay(); | ||||||||||||||
}); | ||||||||||||||
|
||||||||||||||
$('showExternalAddressesInLogViewerLink').addEvent('click', function(e) { | ||||||||||||||
showLogViewer = true; | ||||||||||||||
LocalPreferences.set('show_log_viewer', showLogViewer.toString()); | ||||||||||||||
updateTabDisplay(); | ||||||||||||||
$("logTabLink").click(); | ||||||||||||||
$("logLevelSelect").options[2].selected = true; | ||||||||||||||
window.qBittorrent.Log.logLevelChanged(); | ||||||||||||||
$("filterTextInput").value = 'QBT_TR(Detected external IP.)QBT_TR[CONTEXT=MainWindow]'; | ||||||||||||||
window.qBittorrent.Log.filterTextChanged(); | ||||||||||||||
}); | ||||||||||||||
|
||||||||||||||
const updateTabDisplay = function() { | ||||||||||||||
if (showRssReader) { | ||||||||||||||
$('showRssReaderLink').firstChild.style.opacity = '1'; | ||||||||||||||
|
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.
Doesn't look like you need to remove the
const
: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.
I thought I added that back with the latest commit.