Skip to content

Move TCP connection away from Mac Network lib over to swift-nio enabling - #5

Merged
stensoosaar merged 8 commits into
stensoosaar:mainfrom
TradeWithIt:feat/move-connection-to-nio
Apr 25, 2024
Merged

Move TCP connection away from Mac Network lib over to swift-nio enabling#5
stensoosaar merged 8 commits into
stensoosaar:mainfrom
TradeWithIt:feat/move-connection-to-nio

Conversation

@shial4

@shial4 shial4 commented Apr 24, 2024

Copy link
Copy Markdown
Collaborator

Move TCP connection away from Mac Network lib over to swift-nio enabling, by doing this we enable framework to work wherever Swift does. such as Linux, Windows, even server side.

@stensoosaar
I've tried to keep everything as you had with modifying the connection layer in most.
I haven't had a chance to test it with the IB as IU will get my access back after 15th next month. I'm counting here on your help.

I will leave comments below to address what I think is important.
Let me know what you think :)!

return decoded
}

private func readFrame(buffer: inout ByteBuffer) throws -> ByteBuffer? {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not sure if I did implemented correctly frames decoding, but I left some comments here. Let me know how does it look.

This handler is mostly responsible for reading data and packaging them up into frames.

send(data: greeting)
}

private func stateDidChange(to state: IBConnection.State) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

As this is not Network from MacOs we manage the state ourselves. Not sure if you want to do anything with it

_ = channel.writeAndFlush(buffer)
}

public func receiveMessage(_ data: Data) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is being called, by our 2nd Handler that handles frames from the 1st handler. If that make sense.

and I understand we 1st need to get the initial frame after the greeting. otherwise we read data as usual

import Foundation
import NIOCore

class IBMessageHandler: ChannelInboundHandler {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is our massage handler, that simply converts frames in data and we pass them to our IBConnection

@stensoosaar

Copy link
Copy Markdown
Owner

To get us into same page i included your proposal as NIOClient etc into the dev branch. Hopefully didn't miss anything crucial. At the moment it seems to stuck at opening connection though and never reach connected state to write the greeting message.

@shial4

shial4 commented Apr 25, 2024

Copy link
Copy Markdown
Collaborator Author

@stensoosaar
Yea, I've fixed it already. Pull the recent changes. Although it disconnects now when I try to send any message. Like subscribe to real time data

@shial4

shial4 commented Apr 25, 2024

Copy link
Copy Markdown
Collaborator Author

@stensoosaar let me convert this PR into draft. Till I fix the connection so it is reliable.

@shial4
shial4 marked this pull request as draft April 25, 2024 11:11
@shial4
shial4 marked this pull request as ready for review April 25, 2024 12:04
@shial4

shial4 commented Apr 25, 2024

Copy link
Copy Markdown
Collaborator Author

@stensoosaar PR ready for review :)

connection is working!

Screenshot 2024-04-25 at 10 04 57 PM Screenshot 2024-04-25 at 10 04 32 PM

}

let lengthPrefix = buffer.getInteger(at: buffer.readerIndex, as: Int32.self)!
let frameLength = Int(lengthPrefix.littleEndian)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

FYI:
I had error here, thought it is bigEndian, but it does appear to be littleEndian

case initializing
case connecting(String)
case connected
case connectedToAPI

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

also added, state indicating we are connected to the API

@stensoosaar
stensoosaar merged commit 912fe3b into stensoosaar:main Apr 25, 2024
@stensoosaar

Copy link
Copy Markdown
Owner

Many thanks! Its working! One more thing though... Could you review the connection locking logic, as there seems to be outgoing message race between user requests and handshaking process...

connecting: 127.0.0.1:4002
connected

1714051808.505105.    ("API\0\0\0\0\tv142..175")
1714051808.5111098.   ("\0\0\0G20\00\00\0ETH\0CRYPTO\0\00.00\0\00.00\0PAXOS\0\0USD\0\0\00\0\01 min\060 S\00\0TRADES\01\01\0\0")
connected to API
1714051808.526440.   ("\0\0\0\n71\02\0999\0\0")

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.

2 participants