Updated v2 fork with latest libraries, bug fixes and vcpkg manifest integration

[sukesh-ak]

Updated v2 with latest libraries, bug fixes and vcpkg manifest integration

I have tested on Ubuntu for now.
It should work on all platforms as well viz (Windows, Linux & Mac).

Completed tasks

• Removed external dependency of submodules
• Added vcpkg as submodule with manifest mode ( vcpkg.json )
• Upgraded all the dependencies to latest version - fmt,picosha2,rapidjson,cpp-httplib
• Fixed few bugs connected to fmt upgrade to fmt 9.x
• Updated Kite related header files with proper header file path for vcpkg changes
• Updated httplib error code to strings which was marked as TODO
• Updated CMakeLists.txt with find_package for components coming from vcpkg.

Have added vcpkg as cmake toolchain here in CMakeLists.txt

Clone, compile and test

# Clone recursive v2 branch
$ git clone --recursive https://github.com/Zdhaa/cppkiteconnect -b v2

# pre-compiled (Ubuntu x64) libuWS.a is in uWS folder - since this is an older version with client support
$ cmake . -B build -DBUILD_EXAMPLES=On -DUWS_LIB=uWS/libuWS.a && cmake --build build

Here is the content of vcpkg.json

{
    "name": "cppkiteconnect",
    "version-string": "2.0.0",
    "homepage": "https://github.com/zerodha/cppkiteconnect",
    "description": "C++ Kite Connect API library / SDK",
    "dependencies": [
      "fmt",
      "cpp-httplib",
      "picosha2",
      "rapidjson",
      "zlib",
      "gtest",
      "libuv",
      "openssl"
    ]
  }

For updating the C++ standard to 20 (now it's 17), there are 2 more bugs explained below.

CXX 20 is required since chrono and fmt from CXX 20 onwards supports dates and there are quite a few useful functions for use with this SDK.

std::chrono::sys_days month_last_Thursday(unsigned m, int y)
{
     return {year{y} / month{m} / Thursday[last]};
}

Compiler Errors with CXX 20

In file included from /home/cuteghost/dev/cppkiteconnect/include/kitepp/kite/kite.hpp:28,
                 from /home/cuteghost/dev/cppkiteconnect/include/kitepp.hpp:39,
                 from /home/cuteghost/dev/cppkiteconnect/examples/example1.cpp:26:
/home/cuteghost/dev/cppkiteconnect/include/kitepp/kite/api.hpp: In member function ‘std::string kiteconnect::kite::loginURL() const’:
/home/cuteghost/dev/cppkiteconnect/include/kitepp/kite/api.hpp:45:15: error: ‘this’ is not a constant expression
   45 |     return FMT(loginUrlFmt, "api_key"_a = key);
In file included from /home/cuteghost/dev/cppkiteconnect/include/kitepp/ticker.hpp:28,
                 from /home/cuteghost/dev/cppkiteconnect/include/kitepp.hpp:41,
                 from /home/cuteghost/dev/cppkiteconnect/examples/example1.cpp:26:
/home/cuteghost/dev/cppkiteconnect/include/kitepp/ticker/internal.hpp: In member function ‘void kiteconnect::ticker::connectInternal()’:
/home/cuteghost/dev/cppkiteconnect/include/kitepp/ticker/internal.hpp:170:20: error: ‘this’ is not a constant expression
  170 |     hub.connect(FMT(connectUrlFmt, key, token), nullptr, {},
gmake[2]: *** [CMakeFiles/example1.dir/build.make:76: CMakeFiles/example1.dir/examples/example1.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:89: CMakeFiles/example1.dir/all] Error 2

Bug fix - change to #define

# 1. Change this in kite.hpp
const string loginUrlFmt = "https://kite.zerodha.com/connect/login?v=3&api_key={api_key}";

# To this
#define loginUrlFmt "https://kite.zerodha.com/connect/login?v=3&api_key={api_key}"

# 2. Change this in ws.hpp
const string connectUrlFmt ="wss://ws.kite.trade/?api_key={0}&access_token={1}";

# To this
#define connectUrlFmt "wss://ws.kite.trade/?api_key={0}&access_token={1}"

[bhumitattarde]

Again, thank you for your work!

Removed external dependency of submodules

Correct me if I'm wrong, but won't this make vcpkg binding on users? I'd like to avoid that.

CXX 20 is required

C++ 20 support is still not common so let's avoid this too.

[sukesh-ak]

Again, thank you for your work!

Removed external dependency of submodules

Correct me if I'm wrong, but won't this make vcpkg binding on users? I'd like to avoid that.

Yes and vcpkg integration is not so important but optional. Conan mentioned here has python dependency as I understand so I avoid it.
Requirement is to upgrade the dependency to latest version (simple words, switch the branch to latest in submodules).

I understand the issue with uWebsocket but since Client support is now available it can be upgraded but need not be a top priority for now.

• PicoSHA2 is from 2018. You can see numerous deprecated messages in your run here
• fmt is from 2020 - This does not support date and several other features.
  Since fmt takes care of CXX standards, upgrade to new version is not a breaking change.
• cpp-httplib is from June 2021 - Deprecated messages here too
  New version already gives a function to convert error to string httplib::to_string(res.error())
• rapidjson is from Dec 2020

CXX 20 is required

C++ 20 support is still not common so let's avoid this too.

Is there a reason the URL for WS & HTTP endpoint is a const string as a private variable inside the class?
The fix suggested will work in CXX 17 and also with CXX 20, so it's a good idea to address it now with v2 anyways.

Having said that, it's not important for me to merge this since I can maintain my fork separately as well. But since v2 is new with some breaking changes, it's a good idea to update the old components.

Btw, I used currently released SDK version and tested in September (personal project).
Processed 9 million ticks of 2000+ instruments in a single day on a 2017 gaming laptop running Ubuntu with single websocket connection. Entire infrastructure running in docker containers.
Got busy after that and so getting back now and will be using SDK v2 version this time.

[bhumitattarde]

Yes and vcpkg integration is not so important but optional. Conan mentioned #18 has python dependency as I understand so I avoid it.

With your changes, there would be no way to build the library without vcpkg. I'd like to make sure the project compiles without using any package manager. IMO, package managers should be add-ons and fundamental build system should always be preserved.

Requirement is to upgrade the dependency to latest version (simple words, switch the branch to latest in submodules).

Does this mean latest version or latest master commit? I can definitely get behind the former as long as we pin the versions to make sure build doesn't start failing because a dependency published a version with breaking changes.

Is there a reason the URL for WS & HTTP endpoint is a const string as a private variable inside the class?
The fix suggested will work in CXX 17 and also with CXX 20, so it's a good idea to address it now with v2 anyways.

No particular reason apart from the fact that macros are inferior. However, I do not mind using them here to enable upgrading fmt.

[sukesh-ak]

Once your v2 branch is ready, will send you a PR without package manager and using latest releases of the libraries.

Though its easy to do that yourself too.
Even the master branch of the submodules work, so you can take the latest releases instead.

[bhumitattarde]

I just upgraded the dependencies. Thanks!

[sukesh-ak]

I just upgraded the dependencies. Thanks!

Will take the latest tomorrow and test it.
Did some test today using earlier v2 and got some 7 million ticks without issues.

Is there \r getting added to the HTTP API now?
Talking about the output of this. Thought it was only \n at the end.

curl "https://api.kite.trade/instruments" -H "X-Kite-Version: 3" -H "Authorization: API_KEY:ACCESS_TOKEN" > full.csv

[sukesh-ak]

@bhumitattarde
You can also use new version function to convert error to string httplib::to_string(res.error())

[bhumitattarde]

Is there \r getting added to the HTTP API now?
Talking about the output of this. Thought it was only \n at the end.
curl "https://api.kite.trade/instruments" -H "X-Kite-Version: 3" -H "Authorization: API_KEY:ACCESS_TOKEN" >

@rhnvrm can answer this.

You can also use new version function to convert error to string httplib::to_string(res.error())

Already done

[sukesh-ak]

Closing this PR since main requirements got addressed in v2 now.