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

New incoming message code missing from twsIDs.R ? #19

Closed
PrivacyMatter opened this issue Sep 3, 2019 · 14 comments
Closed

New incoming message code missing from twsIDs.R ? #19

PrivacyMatter opened this issue Sep 3, 2019 · 14 comments
Labels

Comments

@PrivacyMatter
Copy link

Hi,

I noticed that commit a9ae056 introduced support for new incoming message types, such as MARKET_DATA_TYPE, COMMISSION_REPORT, etc (codes 58:68) and they are handled in processMsg.R, e.g:

  else if (curMsg == .twsIncomingMSG$MARKET_DATA_TYPE) {
    msg <- readBin(con, "character", 3)
    eWrapper$marketDataType(curMsg, msg, timestamp, file, 
      ...)
  }
  else if (curMsg == .twsIncomingMSG$COMMISSION_REPORT) {
    msg <- readBin(con, "character", 7)
    eWrapper$commissionReport(curMsg, msg, timestamp, file, 
      ...)
  }

but I do not see these new message codes in twsIDs.R:

`.twsIncomingMSG` <-
structure(list(TICK_PRICE = "1", TICK_SIZE = "2", 
    ORDER_STATUS = "3", ERR_MSG = "4", OPEN_ORDER = "5", ACCT_VALUE = "6", 
    PORTFOLIO_VALUE = "7", ACCT_UPDATE_TIME = "8", NEXT_VALID_ID = "9", 
    CONTRACT_DATA = "10", EXECUTION_DATA = "11", MARKET_DEPTH = "12", 
    MARKET_DEPTH_L2 = "13", NEWS_BULLETINS = "14", MANAGED_ACCTS = "15", 
    RECEIVE_FA = "16", HISTORICAL_DATA = "17", BOND_CONTRACT_DATA = "18", 
    SCANNER_PARAMETERS = "19", SCANNER_DATA = "20", TICK_OPTION_COMPUTATION = "21", 
    TICK_GENERIC = "45", TICK_STRING = "46", TICK_EFP = "47", 
    CURRENT_TIME = "49", REAL_TIME_BARS = "50", FUNDAMENTAL_DATA = "51",
    CONTRACT_DATA_END = "52", OPEN_ORDER_END = "53",
    ACCT_DOWNLOAD_END = "54", EXECUTION_DATA_END = "55",
    DELTA_NEUTRAL_VALIDATION = "56", TICK_SNAPSHOT_END = "57"), .Names = c("TICK_PRICE", 
"TICK_SIZE", "ORDER_STATUS", "ERR_MSG", "OPEN_ORDER", "ACCT_VALUE", 
"PORTFOLIO_VALUE", "ACCT_UPDATE_TIME", "NEXT_VALID_ID", "CONTRACT_DATA", 
"EXECUTION_DATA", "MARKET_DEPTH", "MARKET_DEPTH_L2", "NEWS_BULLETINS", 
"MANAGED_ACCTS", "RECEIVE_FA", "HISTORICAL_DATA", "BOND_CONTRACT_DATA", 
"SCANNER_PARAMETERS", "SCANNER_DATA", "TICK_OPTION_COMPUTATION", 
"TICK_GENERIC", "TICK_STRING", "TICK_EFP", "CURRENT_TIME", "REAL_TIME_BARS",
"FUNDAMENTAL_DATA","CONTRACT_DATA_END", "OPEN_ORDER_END", "ACCT_DOWNLOAD_END",
"EXECUTION_DATA_END","DELTA_NEUTRAL_VALIDATION","TICK_SNAPSHOT_END"
))

I also checked the value of .twsIncomingMSG after installing the latest version and these message indeed seems to be missing, even though the processMsg function does reference them (probably R lazy evaluation is what still allows the code to be built).
Am I missing something, or that these codes were indeed left out?

Thanks.

@joshuaulrich
Copy link
Owner

Hi @PrivacyMatter. Thanks for the report. I may very well have missed that change. As I mentioned in the commit message, I was not able to easily identify the functional changes because of all the formatting changes.

I just pushed the two branches I used to compare (1) the version of master Soren's PR was based on, with (2) the changes in his PR. I would really appreciate it if you could take a look at them and see if you can verify that I indeed missed something he changed.

@PrivacyMatter
Copy link
Author

Hi,
Ok, I took a look, and something definitely happened there. I'm far from being a git expert, so it's not clear to me exactly how and what happened, but I'll describe what I'm seeing:

  1. commit 113ec06: twsIDs has some changes comparing to the parent (on master), including the additional message ids.
  2. the following commit on the same branch, d3bf2d3, does not modify twsIDs. Still, the file is missing when I checkout that version.
  3. back on master, the main commit that introduces the changes, a9ae056, shows no changes too twsIDs.

The one clue that I noticed is that when I compare twsIDs on the branch used as the master for Soren's PR to the first version on the branch you used for applying the changes, the file appears in a "renamed" state. Maybe something happened there that I'm missing, but the bottom line is that the changes you made on 113ec06 disappeared together with the file by a following commit on the same branch.


A possibly related issue: after pulling the latest version on master, I encountered another problem: I could no longer connect to the server (I'm using the latest IB Gateway version). Some investigation of the logs in gateway indicated that it is probably a mismatch in the memory representation of some structures. Changing CLIENT_VERSION <- "63" in twsConnect function (twsConnect.R) to the older "47" value solves the problem.
I noticed now that some of the missing changes in twsIDs included the following lines:

`.IBAPI.VERSION` <- 'clientV63'   #client Version 63

I was hoping that introducing this line by applying the changes in twsIDs.R will solve the problem, but it doesn't look like it does. Even after I copied the version of twsIDs from commit 113ec06 and rebuilt the library, I cannot connect to the server unless I change the CLIENT_VERSION value in twsConnect to be 47.

I will appreciate your input on that.

Thanks,
Yossi

@PrivacyMatter
Copy link
Author

Hi again,

Ok, so it seems that there are many additional problems with the message structure and the connection process, as many changes that were done on the lint-censix branch and were not migrated to the trunk, so using twsIDs.R for version 63 does not work either.
In particular, twsConnect at the head of lint-censix has many changes to twsConnect when comparing it to the branch root --- only one of those is the update of CLIENT_VERSION that was migrated to the trunk (the master branch). It seems that twsConnect was significantly changed on the the lint-censix branch --- e.g. an internal startApi function was introduced, plus many other changes. The only change that was migrated to the trunk is the update to CLIENT_VERSION, though, which may explain why reverting this change back to 47 together with using the old twsIDs.R file worked, even though the rest of the code was modified to support version 63.

At the bottom line, the problem seems to be somewhat more complex than I thought:

  1. With the latest version on the trunk I cannot connect to the server at all. Because of the missing update to twsIDs, though, the structures still match those of client version 47 (without all the new messages), so a workaround is to change twsConnect to identify itself as client version 47.
  2. If I use the correct version of twsIDs.R for version 63, then twsConnect as is on the trunk still doesn't work,I probably due to the many other changes to the twsConnect.R file that were not migrated to the trunk.
  3. Changing CLIENT_VERSION to 47 doesn't work either with the updated version of twsIDs.R because it matches the structures memory footprint of version 63, not 47.

The easiest way to get the library to work for me at the moment is to revert to twsIDs.R of version 47, and change twsConnect to identify itself as client version 47. That does not match the rest of the code on the trunk that assumes version 63, but so far the functionality that I need happens to work despite this mismatch.
I may try to investigate further but I don't have much time for it so will probably use the workaround of using the trunk version and modify CLIENT_VERSION to be 47. Will appreciate if you can comment on that as you obviously know the code better than I do.

@joshuaulrich
Copy link
Owner

Thanks for your detailed analysis! I'll review this weekend.

@joshuaulrich
Copy link
Owner

Thanks again for taking the time to investigate these issues!

The one clue that I noticed is that when I compare twsIDs on the branch used as the master for Soren's PR to the first version on the branch you used for applying the changes, the file appears in a "renamed" state.

Yeah, that is a consequence of how I tried to make the two branches easier to compare. I used package.skeleton() to create the source files. It creates one file for each function. The source code wasn't organized that way, so the contents of the files didn't align with the master branch.

I haven't had time to try and replicate your analysis, but I did do some work to try and make the differences more apparent. I just pushed 3 new branches that call styler::style_dir("R/") on:

  1. the master branch, prior to the PR merge,
  2. Soren's PR branch,
  3. the current master branch (after the merge).

I tried to remove as many of the whitespace changes from Soren's "styled" PR branch. That should make it easier to compare the 3. I haven't tried to compare them yet, and I likely won't have time to do it for a week or so. I'd really appreciate any help!

@PrivacyMatter
Copy link
Author

Hi, will try to get to it. As I mentioned I think there was a problem at least with twsConnect as things didn’t work right with either versions assigned to, but will try to see if that was solved when I have the time to test/look at it. Not sure exactly when, in the midst of some other things.
BTW I happened to meet a guy named William Kats from IB last week in a webinar, and apparently they assigned someone new to work on the API/Gateway front, and asked me for some comments as he meets the guy this week. Didn’t get to this either, but he also said that they have many clients that use R but they all use this library which is how they make them happy. For now . My hope is to try pushing them to support R interface directly. Hopefully your include doesn’t depend on maintaining IBrokers 😄. (I doubt that they will do and definitely not soon, but will try to see what’s possible). Thanks for doing this!
Yossi

@joshuaulrich
Copy link
Owner

I looked at this and... wow, I missed a lot. Please don't spend time investigating until this gets cleaned up. I started working on it yesterday, and will continue to work on it this week.

@PrivacyMatter
Copy link
Author

PrivacyMatter commented Sep 16, 2019 via email

@PrivacyMatter
Copy link
Author

Hi, so i just saw it via github. In

on.exit(if(isOpen(con[[1]])) cancelMktData(con, as.character(as.numeric(tickerId):length(Contract))))

There is using con[[1]] is incorrect, I think. con is defined as conn[[1]] (I.e the actual connection vs the twsConnection object) — see
con <- conn[[1]]
so it should be either con or conn[[1]], I think.

joshuaulrich added a commit that referenced this issue Sep 19, 2019
The handshake between the server and client changed somewhere between
version 47 and 63. Now it needs a START_API message after the client
and server versions have been established.

The server responds to the START_API message with
  1. a managed account message,
  2. a next valid ID, and
  3. messages with the status of the data farms

Soren did not store the server response, so this commit does not
recreate his PR verbatim. One major difference: this change does not
remove the ewrapper, so the next valid ID is still stored in the
ewrapper environment instead of the global environment (as it does in
Soren's PR).

See #19, see #9.
joshuaulrich added a commit that referenced this issue Sep 19, 2019
I attempted to merge all the functional changes from Soren Wilkening's
pull request (#9) in a9ae056.

I missed a lot. This commit attempts to restore the functional changes
I missed. It also adds a few of Soren's comments, including some
blocks of commented code.

See #19.
@joshuaulrich
Copy link
Owner

The two commits I just pushed attempt to restore all the functionality I missed. I'm able to connect to TWS now, and I'm able to pull currency data using the TWS demo.

conn <- twsConnect()
currency <- twsCurrency("EUR")
reqMktData(conn, currency)
TWS Message: 2 1 300 Can't find EId with tickerId:1 
TWS Message: 2 -1 2108 Market data farm connection is inactive but should be available upon demand.usfuture 
TWS Message: 2 -1 2108 Market data farm connection is inactive but should be available upon demand.usfuture 
<20190919 07:04:54.339495> id=1 symbol=EUR bidPrice: 1.10704  bidSize: 5000000 
<20190919 07:04:54.340327> id=1 symbol=EUR askPrice: 1.10705  askSize: 2995000 
<20190919 07:04:54.340815> id=1 symbol=EUR bidSize: 5000000 
<20190919 07:04:54.341098> id=1 symbol=EUR askSize: 2995000 
<20190919 07:04:54.341475> id=1 symbol=EUR lastSize: 0 
<20190919 07:04:54.341817> id=1 symbol=EUR highPrice: 1.10740 
<20190919 07:04:54.342205> id=1 symbol=EUR lowPrice: 1.10230 
<20190919 07:04:54.342559> id=1 symbol=EUR closePrice: 1.10300 
<20190919 07:04:54.343188> id=1 symbol=EUR Halted: 0.0 

That's all I was able to test this morning. I'd appreciate any testing you can do. Thanks!

@PrivacyMatter
Copy link
Author

Thanks Joshua. I am in the midst of extending my use of the library for quite a bit, adding code that also put orders, etc (yes, testing on paper account first); the plus side is that I'll hopefully be able to do quite a deeper / more thorough testing, but OTOH it will probably take a few days till I get to it.

One thing that I noticed is that the bug I mentioned in my previous message is still there (using con[[1]] instead of con or conn[[1]]):

on.exit(if(isOpen(con[[1]])) cancelMktData(con, as.character(as.numeric(tickerId):length(Contract))))

This bug does fire for me, not sure why it doesn't on your tests -- am I missing something?

@joshuaulrich
Copy link
Owner

joshuaulrich commented Sep 20, 2019

I didn't test that line yesterday. I tested it now, and it does not throw an error for me. I agree that it looks like con[[1]] should be con or conn[[1]]. That said, I cannot replicate an issue. Can you give me a small example that causes the behavior, and the output/error you see?

In my tests, con[[1]] returns the number of the connection in the output of showConnections(TRUE). And isOpen() seems to be happy with an integer value for the connection, instead of the connection object itself.

EDIT: My comment isn't meant to suggest that I won't make your suggested change. I probably will.
But I would like to understand the problem.

@PrivacyMatter
Copy link
Author

  • Hmm, I cannot really recall the exact scenario that triggered it, only that it was used in a snapshot and non playback mode. The error message was about "$" object not being subscribe.

  • More importantly, though, I think that I was wrong saying that this bug is the cause for the error that I got, as I've done some other changes due to other issues I had to work around, and I think I commented out some code that probably is the reason for the error that I got;

    • I mistakenly commented the following code at the beginning:
     -    if (!is.twsConnection(conn))
     -        stop("tws connection object required")
     -
     -    if(!is.twsPlayback(conn)) {
     -      Contract <- as.twsContract(Contract)
     -      if(is.twsContract(Contract)) Contract <- list(Contract)
     -  
     -      for(n in 1:length(Contract)) {
     -        if (!is.twsContract(Contract[[n]])) 
     -            stop("twsContract required")
     -      }
     -      
     -    } #else file <- ""
     +    # if (!is.twsConnection(conn))
     +    #     stop("tws connection object required")
     +    #
     +    # if(!is.twsPlayback(conn)) {
     +    #   Contract <- as.twsContract(Contract)
     +    #   if(is.twsContract(Contract)) Contract <- list(Contract)
     +    #     
     +    #   for(n in 1:length(Contract)) {
     +    #     if (!is.twsContract(Contract[[n]]))
     +    #         stop("twsContract required")
     +    #   }
     +    #
     +    # } #else file <- ""
    

    when I dealt with the issue with playback that you (and I, locally) later fixed. Then I applied only the fix of the con[[1]] vs con bug on a fresh copy of the function, and it made it seem as if the problem is due to that bug (I basically had two versions of the library, one with only that fix and one with the other buggy workaround but without that fix, which was the source of my confusion).

    Really sorry for the confusion I caused. While it is indeed a bug that might manifest at some point (there are no guarantees that the structure of R connection object or the implementation of isOpen will remain the same), it's definitely not as important to address it as I thought. It's probably still a good idea to fix it regardless before the next push to the trunk, and hopefully to CRAN.

As for testing: writing the code that uses the library more heavily takes a bit longer than expected (as these things always do); I do hope to have some code that uses the library to monitor and make actual trades with a paper account at some point during next week. That's my highest propriety at the moment in the project I'm working on. Once I have that, it will be a good opportunity to test the latest version on the branch.

@joshuaulrich
Copy link
Owner

  • Really sorry for the confusion I caused.

No worries! And thanks for the detailed explanation. I'm going to make your suggested change and close this issue. Please open new issues for new problems you find (one issue per problem).

Thanks again for all your contributions! You have been incredibly helpful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants