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

Expose servus_host and servus_port keys #82

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

eile
Copy link
Contributor

@eile eile commented Feb 22, 2018

No description provided.

@bbpbuildbot
Copy link

Can one of the admins verify this patch?

@eile
Copy link
Contributor Author

eile commented Feb 22, 2018

Gah, hold on a sec. The pre-commit clang-format mangled this one.

@eile eile force-pushed the master branch 4 times, most recently from 8054efe to 8117ff0 Compare February 22, 2018 11:24
@eile
Copy link
Contributor Author

eile commented Feb 22, 2018

Good to go

@janbajanadaqri
Copy link

@eile Thank you for PR. This really helps us.

Copy link

@janbajanadaqri janbajanadaqri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR helps us to get currently missing port during browsing. Thank you.

@tribal-tec
Copy link
Contributor

retest this please

@tribal-tec
Copy link
Contributor

FAILED: /usr/lib/ccache/c++   -DBOOST_ALL_NO_LIB -DBoost_NO_BOOST_CMAKE -DLinux=1 -DSERVUS_DSO_NAME=\"libServus.so.1.6.0\" -DSERVUS_LITTLEENDIAN=1 -DSERVUS_SHARED -DSERVUS_USE_AVAHI_CLIENT=1 -DSERVUS_USE_BOOST=1 -DSERVUS_USE_CXX11=1 -DSERVUS_USE_QT5CORE=1 -DSERVUS_USE_QT5WIDGETS=1 -DSERVUS_USE_THREADS=1 -DServus_EXPORTS -DUSE_PYTHON3=1 -DWARN_DEPRECATED -I/usr/include/x86_64-linux-gnu/qt5 -I/usr/include/x86_64-linux-gnu/qt5/QtCore -I/usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++-64 -I/usr/include/x86_64-linux-gnu/qt5/QtWidgets -I/usr/include/x86_64-linux-gnu/qt5/QtGui -I../ -Iinclude -I. -g -fPIC   -Wnon-virtual-dtor -Wsign-promo -Wvla -fno-strict-aliasing -Wall -Wextra -Winvalid-pch -Winit-self -Wno-unknown-pragmas -Wshadow -Werror -fmax-errors=5 -std=gnu++11 -MMD -MT servus/CMakeFiles/Servus.dir/servus.cpp.o -MF servus/CMakeFiles/Servus.dir/servus.cpp.o.d -o servus/CMakeFiles/Servus.dir/servus.cpp.o -c ../servus/servus.cpp
In file included from ../servus/servus.cpp:183:0:
../servus/avahi/servus.h: In member function ‘void servus::avahi::Servus::_resolveCB(AvahiServiceResolver*, AvahiResolverEvent, const char*, const char*, uint16_t, AvahiStringList*, AvahiLookupResultFlags)’:
../servus/avahi/servus.h:346:56: error: passing ‘uint16_t {aka short unsigned int}’ chooses ‘int’ over ‘unsigned int’ [-Werror=sign-promo]
             values["servus_port"] = std::to_string(port);
                                                        ^
../servus/avahi/servus.h:346:56: error:   in call to ‘std::__cxx11::string std::__cxx11::to_string(int)’ [-Werror=sign-promo]
In file included from ../servus/servus.cpp:186:0:
../servus/test/servus.h: In member function ‘virtual servus::Servus::Result servus::test::Servus::browse(int32_t)’:
../servus/test/servus.h:106:57: error: passing ‘short unsigned int’ chooses ‘int’ over ‘unsigned int’ [-Werror=sign-promo]
             values["servus_port"] = std::to_string(_port);
                                                         ^
../servus/test/servus.h:106:57: error:   in call to ‘std::__cxx11::string std::__cxx11::to_string(int)’ [-Werror=sign-promo]

@janbajanadaqri
Copy link

janbajanadaqri commented Feb 22, 2018

@eile I have verified your PR between MAC and Linux in my fork, branch "host_port_test". Apps "SampleAnnounce" and "SampleBrowserAsync". All works perfectly.
https://github.com/janbajanadaqri/Servus/tree/host_port_test

my result from SampleBrowserAsync:

Service type: _daqri-test._tcp
Add
  Service name: daqri.core.124875fd81b8b92f:7c3275875be18e1c
  Address: JanBajana-MBP2-IR1401.local
  TXT: UUID=124875fd81b8b92f:7c3275875be18e1c
  TXT: priority=1
  TXT: servus_host=JanBajana-MBP2-IR1401.local
  TXT: servus_port=2366

Going to try Windows as well.

@janbajanadaqri
Copy link

janbajanadaqri commented Feb 22, 2018

Modified:
@tribal-tec same issue but only gcc.
Compilation error when building with "gcc". I had to change "servus_port" from:

values["servus_port"] = std::to_string(port);

to

values["servus_port"] = std::to_string(static_cast<unsigned int>(port));

This error doesn't appear with "clang" neither "msvc" in windows.

using clang version 3.8.0-2ubuntu4 (tags/RELEASE_380/final)
using gcc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
GCC Error:

/home/dslocal/Work/CLion/Servus/servus/avahi/servus.h: In member function 'void servus::avahi::Servus::_resolveCB(AvahiServiceResolver*, AvahiResolverEvent, const char*, const char*, uint16_t, AvahiStringList*, AvahiLookupResultFlags)':
/home/dslocal/Work/CLion/Servus/servus/avahi/servus.h:346:68: error: passing 'uint16_t {aka short unsigned int}' chooses 'int' over 'unsigned int' [-Werror=sign-promo]
                         values["servus_port"] = std::to_string(port);
/home/dslocal/Work/CLion/Servus/servus/avahi/servus.h:346:68: error:   in call to 'std::__cxx11::string std::__cxx11::to_string(int)' [-Werror=sign-promo]

@janbajanadaqri
Copy link

janbajanadaqri commented Feb 23, 2018

@eile issue:
Another issue with the port number in Bonjour (Windows, MAC). The returned port number is wrong because it is in network byte order from DNSServiceResolve();
The solution is big-endian byte conversation in "dnssd/servus.h" line 312:

uint16_t bigEndianPort = ((port & 0x00ff) << 8) | ((port & 0xff00) >> 8);
values["servus_port"] = std::to_string(bigEndianPort);

you can see my branch created from your PR:
https://github.com/janbajanadaqri/Servus/blob/host_port_test/servus/dnssd/servus.h#L312

Tested on MAC and Windows.
But can be this different for other architectures than Intel? May it need a detection for endian?

@hernando
Copy link

Why not using the function htons? It's available in Linux, Mac and Windows

@eile
Copy link
Contributor Author

eile commented Mar 1, 2018

Addressed issues.

@tribal-tec
Copy link
Contributor

retest this please

@tribal-tec
Copy link
Contributor

Test fails on Debug&Release for Ubuntu&RedHat:

 5/13 Test  #5: itemModel ........................***Failed    2.08 sec
Running 2 test cases...
Entering test suite "servus_itemModel"
Entering test case "invalidAccess"
Leaving test case "invalidAccess"
Entering test case "servusItemModel"
../tests/itemModel.cpp(181): fatal error in "servusItemModel": critical check model.rowCount(instanceIndex) == 2 failed [3 != 2]

The test driver instanceRemoved() passed an empty string for the instance name since the object behind the pointer is gone at this point.
@eile
Copy link
Contributor Author

eile commented Mar 8, 2018

@tribal-tec Fixed, and a bit more in dfd765b

@tribal-tec tribal-tec removed their assignment Jul 11, 2019
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.

5 participants