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

Start working on TCP comms #96

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

szymex73
Copy link
Contributor

This change will allow for connecting to a TCP server that exposes the chameleon serial interface, similar to how the PM3 CLI allows for connecting to a TCP server exposing a Proxmark. This should allow for the use of the CLI on devices like Android phones with Termux.

Currently, it's working for communicating with a device but needs a bug fixed with a thread locking up when trying to close the socket.

@github-actions
Copy link

You are welcome to add an entry to the CHANGELOG.md as well

@github-actions
Copy link

Built artifacts for commit b8e11bd

Firmware

Client

@doegox
Copy link
Contributor

doegox commented Aug 26, 2023

Just sharing some thoughts, feel free to ignore, I don't want to make your life harder neither unless you think it's worth investigating...

To also support BLE in CLI, the way seems to use Bleak, which is based on asyncio. If we move to a unified asyncio model for transports, there is also pyserial-asyncio.
Now for TCP, if you use Streams it's also asyncio.

So, maybe should we investigate moving to a unified asyncio API for transport?
(I must confess I've always more difficulties thinking with async than sync, but well...)

@GameTec-live
Copy link
Contributor

Just sharing some thoughts, feel free to ignore, I don't want to make your life harder neither unless you think it's worth investigating...

To also support BLE in CLI, the way seems to use Bleak, which is based on asyncio. If we move to a unified asyncio model for transports, there is also pyserial-asyncio. Now for TCP, if you use Streams it's also asyncio.

So, maybe should we investigate moving to a unified asyncio API for transport? (I must confess I've always more difficulties thinking with async than sync, but well...)

migrating to async might be worth it to prevent future issues, etc.

@szymex73
Copy link
Contributor Author

It would require some rearchitecturing but I do agree that going async might be a better choice with the transaction model we currently have. Could take a look at that pretty soon but can't say for sure

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.

3 participants