mathpaquette / IQFeed.CSharpApiClient

IQFeed.CSharpApiClient is fastest and the most well-designed C# DTN IQFeed socket API connector available
MIT License
121 stars 42 forks source link

Passing requestId to ReqHistoryIntervalTimeframeAsync throws System.FormatException #13

Closed hunterfries closed 6 years ago

hunterfries commented 6 years ago

System.FormatException: 'String was not recognized as a valid DateTime.' is thrown when requestId is passed to ReqHistoryIntervalTimeframeAsync.

mathpaquette commented 6 years ago

Hello @hunterfries, in fact, all request id parameters arent supported because they're not really useful since every requests are made individually on another thread/client or sequentially (await). This mechanism is only supported by IQFeed to distinguish data coming from many requests executed at the same time but not applicable for us.

I will remove this parameter from all lookup requests pretty soon.

mathpaquette commented 6 years ago

@hunterfries Well, not so sure of this conclusion for Derivative data... Ill look to a proper solution but looks like possible that request ids will be necessary....

hunterfries commented 6 years ago

@mathpaquette Sounds good. I don't think that changes removing them from lookup requests. I was already in a request id mindset, which is how I found this in the first place.

mbmurray commented 6 years ago

Hey guys. I'm glad you came to the conclusion that you will need the request ID when looking up historical data. I ran into this problem when trying to use this library in an existing solution that previously used the COM object to communicate with IQ feed. I fixed this in a local copy for my needs but I can share my updates with you if you'd like.

mathpaquette commented 6 years ago

@mbmurray thank you for participating in that discussion. Well, because I have pressure from you guys, I'll add functionality to support parsing with/without request Id. For lookup requests, I feel this is a bit a bad practice but since you have to adapt into an old project it makes sense to support it. For the code it's fine but thank you very much again for your input!

mathpaquette commented 6 years ago

@hunterfries or @mbmurray please review, Ill merge into master