Skip to content

First cut at phone to robot RPC - #1

Open
pmalmsten wants to merge 14 commits into
trc492:masterfrom
DrDab:master
Open

First cut at phone to robot RPC#1
pmalmsten wants to merge 14 commits into
trc492:masterfrom
DrDab:master

Conversation

@pmalmsten

Copy link
Copy Markdown
Collaborator

No description provided.

@pmalmsten pmalmsten changed the title DabDab First cut at phone to robot RPC Aug 15, 2018
/home/citrus/wpilib/user/java/lib/libCTRE_PhoenixCCI.so=8f5e4ab9156fdf558f7805d280dab06a
C\:\\Users\\Michael\\wpilib\\user\\java\\lib\\libCTRE_PhoenixCCI.so=e02aeb19c8a0962f906f1bbbb0100fc6
C\:\\Users\\TrcUser\\wpilib\\user\\java\\lib\\libCTRE_PhoenixCCI.so=e02aeb19c8a0962f906f1bbbb0100fc6
C\:\\Users\\Abhay\\wpilib\\user\\java\\lib\\libCTRE_PhoenixCCI.so=8f5e4ab9156fdf558f7805d280dab06a

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.

I'm curious what this file is for.

*
* @author Artem Prigoda
*/
public class JsonRpcClient {

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.

It would be nice to pull in library code from other teams as jar files instead of as raw source. That would help keep our source tree more minimal, make it easier to navigate, and also make it easier to upgrade library versions over time.

Usually we only import raw source for libraries if we need to alter the library ourselves for some reason and can't or don't want to maintain our own builds of it.


public AdbBridge(Path location)
{
bin_location_ = location;

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.

I see that there are two different ways to override the path to the binary - either via this second constructor, or the first constructor via the environment variable. Any idea which way people would prefer to change the path? I'm curious if we need both ways to do that.


import com.github.arteam.simplejsonrpc.client.Transport;

public class AwooCommunicator implements Transport

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.

Cute name.

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.

We might be able to skip writing all of this code if we were to use jsonrpc4j instead - it looks like it offers a SocketServer that can work with InputStream and OutputStreams directly, which would pair up nicely with the Socket provided by ADB.

I've only looked very briefly, so probably needs some investigation - here's a link: https://github.com/briandilley/jsonrpc4j

{
private int port;
private BufferedWriter bw;
private Socket kemono;

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 a bit unusual of a name for a socket.

Comment thread src/team492/Robot.java
awooCommunicator = new AwooCommunicator(13970);
awooCommunicator.initCommunicator();

rpcClient = new JsonRpcClient(awooCommunicator);

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.

It would be nice to hide more of these setup details outside of Robot.java such that users of the class do not need to know all of these details to get it work.

Comment thread src/team492/Robot.java
diagnostics = new OnBoardDiagnostics(this);


// ayyioasjfasfdjisagoibdsaofisajo;bas;ofjsaiojfasiojgisaofj

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.

That's a rather creative comment.

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