search

brannondorsey / chattervox

📡 An AX.25 packet radio chat protocol with support for digital signatures and binary compression. Like IRC over radio waves.
Other
748 stars 36 forks source link

Add KISS support over TCP #7

Closed brannondorsey closed 5 years ago

brannondorsey commented 5 years ago

For cross-platform compatibility with direwolf's kiss server running on port 8001.

ve1jot commented 5 years ago

Actually, both agw and kiss ports are configurable in the D.W. config file, for instance, mine are at 8010-8011

brannondorsey commented 5 years ago

Yep! The kiss support over TCP is one of Direwolf's best features IMHO. Currently though, the TypeScript client I've written doesn't take advantage of this yet. I'm borrowing from @echicken's kiss-tnc to pass ax25 packets back and forth via KISS over a serial interface only. I intend to (and would also accept a PR if someone wants to jump on it first) implement this communication over TCP in addition to a serial connection, and that's what this issue is for.

danc256 commented 5 years ago

I started working on this 2 days ago. I found Utils for APRS and it looks like it supports both serial and TCP implementations for KISS. It mentions Dire Wolf by name so I'm pretty confident it'll work. Might also make sense to refactor the existing serial implementation to use the same library since it supports both transports but that can be a future enhancement. I should have something testable today or tomorrow. @brannondorsey Can I get this issue assigned to me?

brannondorsey commented 5 years ago

I was just getting caught up on emails and saw you are making great progress in this direction! I've added you as a collaborator to the project, and I see you've already assigned yourself the issue 😸.

Might also make sense to refactor the existing serial implementation to use the same library since it supports both transports but that can be a future enhancement.

I agree that using a unified library for both would be preferable. I did look into Utils for APRS's package.json and its using a version of serialport that is far lower than the KISS-TNC version I'm using, so that's a bit disconcerting. It also lists the serialport dependency as a dev dependency, so I'm not sure what's up with that 😕.

Sorry I haven't pushed my half-way there TCP/IP KISS-TNC version yet, I still need to dig that up. In the meantime, carry on, sounds like you are making good progress nonetheless. I'm a bit hesitant to have two separate dependencies for KISS functionality, but maybe that will be a transitional thing or something.

P.S., I've also added push protection to the master branch, so everything committed there will have to go through the PR process. I've created a new develop branch as well where I will be putting new code versions before merging them to master. You can make PRs against develop and then we'll eventually merge those to master.

danc256 commented 5 years ago

Ah fair point on the serialport functionality. Didn’t catch that. The great thing about the factory method is you can implement both serial libraries (they’re both there anyway) and make an informed choice.

Good deal on the develop branch. I’ll merge that in before I push my feature branch up to my repo. Hopefully it’s got the current master branch merged into it.

Thanks for the access. I’d rather put my progress notes in the issue than spamming your inbox every 5m. I like scalable solutions.

Also, as I read through the Dire Wolf user guide, they specifically point out that using the radio’s VOX circuit for packet radio has some drawbacks. At best it leaves the channel open longer than necessary, and at worst it can be constantly transmitting. I think I’ll open an issue for that and maybe a proposed fix.

From: Brannon Dorsey notifications@github.com Reply-To: brannondorsey/chattervox reply@reply.github.com Date: Saturday, March 9, 2019 at 11:01 AM To: brannondorsey/chattervox chattervox@noreply.github.com Cc: Dan Chisarick chisarickd@doombridge.com, Assign assign@noreply.github.com Subject: Re: [brannondorsey/chattervox] Add KISS support over TCP (#7)

I was just getting caught up on emails and saw you are making great progress in this direction! I've added you as a collaborator to the project, and I see you've already assigned yourself the issue 😸.

Might also make sense to refactor the existing serial implementation to use the same library since it supports both transports but that can be a future enhancement.

I agree that using a unified library for both would be preferable. I did look into Utils for APRS's package.jsonhttps://github.com/trasukg/utils-for-aprs/blob/develop/package.json#L34 and its using a version of serialport that is far lower than the KISS-TNC version I'm usinghttps://github.com/brannondorsey/kiss-tnc/blob/master/package.json#L15, so that's a bit disconcerting. It also lists the serialport dependency as a dev dependency, so I'm not sure what's up with that 😕.

Sorry I haven't pushed my half-way there TCP/IP KISS-TNC version yet, I still need to dig that up. In the meantime, carry on, sounds like you are making good progress nonetheless. I'm a bit hesitant to have two separate dependencies for KISS functionality, but maybe that will be a transitional thing or something.

P.S., I've also added push protection to the master branch, so everything committed there will have to go through the PR process. I've created a new develop branch as well where I will be putting new code versions before merging them to master. You can make PRs against develop and then we'll eventually merge those to master.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/brannondorsey/chattervox/issues/7#issuecomment-471195230, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ARH_VNlcZPB4fKez8jtrK3Gld49Id82Uks5vU9rbgaJpZM4W10na.

brannondorsey commented 5 years ago

I've just pushed up my midway-progress from several months ago on adding TCP support to the kiss-tnc library. Check out this branch. I haven't looked at the code in several months, but after reviewing it a bit just now it looks like I implemented sending functionality, but not yet receiving functionality. The main things to look at there are example.js and index.js. There is only one commit that I've added so you should be able to see the diff pretty easily.

From the looks of it, if we wanted to add KISS support over TCP using the existing library chattervox is using, we'd just have to implement this stuff via TCP, similarly to how I've done that here for TX.

Once we've got TCP support for KISS, we can add some new settings in the ~/.chattervox/config.json file for network support. Something like:

{
    "version": 3,
    "callsign": "KC3LZO",
    "ssid": 0,
    "keystoreFile": "/home/braxxox/.chattervox/keystore.json",
    "kissPort": "/tmp/kisstnc",
    "kissTCPPort": 8001,
    "kissTCPHost": "127.0.0.1", // we'll add this
    "kissBaud": 9600, // and this
    "feedbackDebounce": 20000,
    "signingKey": "044da0d4c38bed6e5bc418231cb2dca4f690d858d36c38a032732553b76262a1adfccf588b6c1f9d7734b1bbce90914f82"
}

I’d rather put my progress notes in the issue than spamming your inbox every 5m.

I like this too. I prefer public discussion on GitHub anyway, in case people want to chime in or read through past issues to learn more about how design decisions came to be.

Also, as I read through the Dire Wolf user guide, they specifically point out that using the radio’s VOX circuit for packet radio has some drawbacks. At best it leaves the channel open longer than necessary, and at worst it can be constantly transmitting. I think I’ll open an issue for that and maybe a proposed fix.

Yeah, this is a known drawback to AX.25 over VOX in general. The reason that I suggest it in the documentation, and in my workshop, is that its just saw dang cheap and easy. VOX allows for the quick and dirty setup, where you don't need anything but a simple and a radio. But now we are getting into a domain that is actually not a part of Chattervox. Chattervox is simply the client, it's up to the TNC and the radio to decide what transmission modes and settings to use.

To the same end, following up on something you said over email a few days ago...

I think cleaning up the Direwolf interface across all platforms and generally making that linkage easier would go a long way towards improving usability. For example, automatically starting Direwolf if it is detected in the path but not actually running. If Chattervox started it then it should stop it on exit, but if it was already running it should be left alone. There are 1k other ideas. Start with this one.

Having Chattervox launch and manage Direwolf as a child process is actually something that I was building in the early days of Chattervox, but decided to intentionally remove. I made this decision to inline with the UNIX philosophy that Chattervox shouldn't try and do too many things. You and I use Dirwolf, but many people have their own TNCs, and I didn't want to link Chattervox to Dirwolf, only suggest that they may be used together. I also was having quite a bit of trouble managing the Dirwolf process in a sane way, resulting in many edge cases and possible zombie processes depending on different exception types. That's when I decided to instead warn if no TNC serial port is present on Chattervox launch, and give a soft suggestion for a command to fix it with direwolf -q d -t 0. For now, keeping the process management of the two applications separate is still something that I agree with. Especially so that the user learns the very distinct and separate roles the chattervox client plays from the TNC. I also see it as a learning opportunity :)

Thanks again for your interest and continued help man. I'm excited by the energy you are bringing to the project.

danc256 commented 5 years ago

Huh. It might make sense to just stuff the guts from your work in the TCP handler. That way your effort is fully leveraged. There is no code overlap between what you or I did.

My primary focus is separating Messenger from the underlying protocol, and secondary is to make a loopback protocol to make end-to-end testing easier.

If you want to get fancy, you can have a composite handler that chains two handlers sequentially. That way you can chain the TCP or Serial version with the loopback version. Since the loopback processing is a callback, now you can make bots or something like ‘Eliza’.

Separating these concerns opens up a lot of creative possibilities with a small effort. Chain the loopback protocol after the receive handler so it can process the input, then chain the loopback sender before the real sender to allow procedural responses. Add a source flag so it can differentiate between interactive input and procedural. Barrels of fun.

From: Brannon Dorsey notifications@github.com Reply-To: brannondorsey/chattervox reply@reply.github.com Date: Saturday, March 9, 2019 at 11:38 AM To: brannondorsey/chattervox chattervox@noreply.github.com Cc: Dan Chisarick chisarickd@doombridge.com, Assign assign@noreply.github.com Subject: Re: [brannondorsey/chattervox] Add KISS support over TCP (#7)

I've just pushed up my midway-progress from several months ago on adding TCP support to the kiss-tnc library. Check out this branchhttps://github.com/brannondorsey/kiss-tnc/tree/kiss-tcp. I haven't looked at the code in several months, but after reviewing it a bit just now it looks like I implemented sending functionality, but not yet receiving functionality. The main things to look at there are example.jshttps://github.com/brannondorsey/kiss-tnc/blob/kiss-tcp/example.js and index.jshttps://github.com/brannondorsey/kiss-tnc/blob/kiss-tcp/index.js. There is only one commit that I've added so you should be able to see the diffhttps://github.com/brannondorsey/kiss-tnc/commit/20d7b9593a744535d92cbae2a9431633c170ce6b pretty easily.

From the looks of it, if we wanted to add KISS support over TCP using the existing library chattervox is using, we'd just have to implement this stuffhttps://github.com/brannondorsey/kiss-tnc/blob/kiss-tcp/index.js#L42-L84 via TCP, similarly to how I've done that here for TXhttps://github.com/brannondorsey/kiss-tnc/blob/kiss-tcp/index.js#L159-L163.

Once we've got TCP support for KISS, we can add some new settings in the ~/.chattervox/config.json file for network support. Something like:

{

"version": 3,

"callsign": "KC3LZO",

"ssid": 0,

"keystoreFile": "/home/braxxox/.chattervox/keystore.json",

"kissPort": "/tmp/kisstnc",

"kissTCPPort": 8001,

"kissTCPHost": "127.0.0.1"

"kissBaud": 9600,

"feedbackDebounce": 20000,

"signingKey": "044da0d4c38bed6e5bc418231cb2dca4f690d858d36c38a032732553b76262a1adfccf588b6c1f9d7734b1bbce90914f82"

}

I’d rather put my progress notes in the issue than spamming your inbox every 5m.

I like this too. I prefer public discussion on GitHub anyway, in case people want to chime in or read through past issues to learn more about how design decisions came to be.

Also, as I read through the Dire Wolf user guide, they specifically point out that using the radio’s VOX circuit for packet radio has some drawbacks. At best it leaves the channel open longer than necessary, and at worst it can be constantly transmitting. I think I’ll open an issue for that and maybe a proposed fix.

Yeah, this is a known drawback to AX.25 over VOX in general. The reason that I suggest it in the documentation, and in my workshop, is that its just saw dang cheap and easy. VOX allows for the quick and dirty setup, where you don't need anything but a simple and a radio. But now we are getting into a domain that is actually not a part of Chattervox. Chattervox is simply the client, it's up to the TNC and the radio to decide what transmission modes and settings to use.

To the same end, following up on something you said over email a few days ago...

I think cleaning up the Direwolf interface across all platforms and generally making that linkage easier would go a long way towards improving usability. For example, automatically starting Direwolf if it is detected in the path but not actually running. If Chattervox started it then it should stop it on exit, but if it was already running it should be left alone. There are 1k other ideas. Start with this one.

Having Chattervox launch and manage Direwolf as a child process is actually something that I was building in the early days of Chattervox, but decided to intentionally remove. I made this decision to inline with the UNIX philosophy that Chattervox shouldn't try and do too many things. You and I use Dirwolf, but many people have their own TNCs, and I didn't want to link Chattervox to Dirwolf, only suggest that they may be used together. I also was having quite a bit of trouble managing the Dirwolf process in a sane way, resulting in many edge cases and possible zombie processes depending on different exception types. That's when I decided to instead warn if no TNC serial port is present on Chattervox launch, and give a soft suggestion for a command to fix it with direwolf -q d -t 0. For now, keeping the process management of the two applications separate is still something that I agree with. Especially so that the user learns the very distinct and separate roles the chattervox client plays from the TNC. I also see it as a learning opportunity :)

Thanks again for your interest and continued help man. I'm excited by the energy you are bringing to the project.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/brannondorsey/chattervox/issues/7#issuecomment-471198663, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ARH_VEuXAna4Gquq0z33MnwHLiXWX1K7ks5vU-NxgaJpZM4W10na.

brannondorsey commented 5 years ago

I actually just spent the hour and a half that I should have spent 5 months ago to add full TCP support to Kiss-TNC. I haven't tested it a ton, but it should be working now!

With this method, we can leave everything as is now, but specify the serial device as kiss://localhost:8001 instead of /tmp/kisstnc! I just need to do a bit more testing and then simply depend on my new version of the kiss-tnc module.

I hope you don't feel like your work is for naught! We can always refactor/choose whichever implementation is better for the code base. This was just what I could whip up quickly with the current dependency.

danc256 commented 5 years ago

Nothing is ever wasted. JavaScript isn’t my specialty so I’m not the fastest at it. It’s a useful learning experience regardless of anything else. When I’m done with this I’ll move it to a branch and you can see what I had in mind. Probably…. Somewhat more impactful than the changes you made.

From: Brannon Dorsey notifications@github.com Reply-To: brannondorsey/chattervox reply@reply.github.com Date: Saturday, March 9, 2019 at 1:20 PM To: brannondorsey/chattervox chattervox@noreply.github.com Cc: Dan Chisarick chisarickd@doombridge.com, Assign assign@noreply.github.com Subject: Re: [brannondorsey/chattervox] Add KISS support over TCP (#7)

I actually just spent the hour and a half that I should have spent 5 months ago to add full TCP support to Kiss-TNChttps://github.com/brannondorsey/kiss-tnc/tree/kiss-tcp. I haven't tested it a ton, but it should be working now!

With this method, we can leave everything as is now, but specify the serial device as kiss://localhost:8001 instead of /tmp/kisstnc! I just need to do a bit more testing and then simply depend on my new version of the kiss-tnc module.

I hope you don't feel like your work is for naught! We can always refactor/choose whichever implementation is better for the code base. This was just what I could whip up quickly with the current dependency.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/brannondorsey/chattervox/issues/7#issuecomment-471208473, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ARH_VLOdjy4oiMtJMaXsojGyTDHx_xUoks5vU_t8gaJpZM4W10na.

danc256 commented 5 years ago

Here’s what I was thinking: https://github.com/danc256/chattervox/commit/617268f9ce6d56a1d431fb3d8dcb84c48de06227

I extracted all the transport functionality into a TNCConnection interface, then made a TNCConnectionFactory to build the various transports based on the value in the config.json

You can define a new configuration property “tncConnectionType”. If you omit it, it defaults to “Serial”. If you set it to “Test” then it will echo back whatever you send after 2 seconds. All defined values are in the “TNCConnectionType” enum in config.ts.

Here's the branch if you want to checkout a copy and run it: https://github.com/danc256/chattervox/tree/kissnetwork

If this wasn’t the direction you wanted to go, no worries. It was an opportunity to mess with Node.js for a day or two. I can leave this where it is and go off and do other things. The bulk of the effort was to refactor Messenger.ts and extract the transport code. Because you were able to enhance the existing library you moved the behavior to the transport library. I think there’s value in separation for testing and stuff like bots. Also the ‘exec’ functionality you added could go in there as well.

LMK your thoughts. Again, no worries whatever you decide.

From: Brannon Dorsey notifications@github.com Reply-To: brannondorsey/chattervox reply@reply.github.com Date: Saturday, March 9, 2019 at 1:20 PM To: brannondorsey/chattervox chattervox@noreply.github.com Cc: Dan Chisarick chisarickd@doombridge.com, Assign assign@noreply.github.com Subject: Re: [brannondorsey/chattervox] Add KISS support over TCP (#7)

I actually just spent the hour and a half that I should have spent 5 months ago to add full TCP support to Kiss-TNChttps://github.com/brannondorsey/kiss-tnc/tree/kiss-tcp. I haven't tested it a ton, but it should be working now!

With this method, we can leave everything as is now, but specify the serial device as kiss://localhost:8001 instead of /tmp/kisstnc! I just need to do a bit more testing and then simply depend on my new version of the kiss-tnc module.

I hope you don't feel like your work is for naught! We can always refactor/choose whichever implementation is better for the code base. This was just what I could whip up quickly with the current dependency.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/brannondorsey/chattervox/issues/7#issuecomment-471208473, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ARH_VLOdjy4oiMtJMaXsojGyTDHx_xUoks5vU_t8gaJpZM4W10na.

danc256 commented 5 years ago

I tested sending and receiving using your sockets enhancement and it looks like it works.

It’s interactive, not exactly cross-platform, but it works. Dire Wolf also has its own utility for creating test WAV files from a CLI tool called “gen_packet”. Not exactly what you want perhaps but it’s another option.

I tried tested your creation in a Windows VM but I’m getting problems accessing the microphone. This is a problem at the VM level as it also fails with other software. I think I hit a natural stopping point on this for now and will move onto other things for a bit.

At any rate, fine work on the TCP implementation. I think the way you want to go about integration testing this in a platform independent manner is to mock a TCP server. There are loads of tools to do this, but one written in Node would be ideal. This way you can stand up a local TCP test server, send a message, verify it was sent, then have the server send a message and verify Chattervox decoded it correctly. Testing this with audio files is a massive pain and is really testing Dire Wolf. Mocking at the socket layer is probably the way to go.

From: Brannon Dorsey notifications@github.com Reply-To: brannondorsey/chattervox reply@reply.github.com Date: Saturday, March 9, 2019 at 1:20 PM To: brannondorsey/chattervox chattervox@noreply.github.com Cc: Dan Chisarick chisarickd@doombridge.com, Assign assign@noreply.github.com Subject: Re: [brannondorsey/chattervox] Add KISS support over TCP (#7)

I actually just spent the hour and a half that I should have spent 5 months ago to add full TCP support to Kiss-TNChttps://github.com/brannondorsey/kiss-tnc/tree/kiss-tcp. I haven't tested it a ton, but it should be working now!

With this method, we can leave everything as is now, but specify the serial device as kiss://localhost:8001 instead of /tmp/kisstnc! I just need to do a bit more testing and then simply depend on my new version of the kiss-tnc module.

I hope you don't feel like your work is for naught! We can always refactor/choose whichever implementation is better for the code base. This was just what I could whip up quickly with the current dependency.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/brannondorsey/chattervox/issues/7#issuecomment-471208473, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ARH_VLOdjy4oiMtJMaXsojGyTDHx_xUoks5vU_t8gaJpZM4W10na.

brannondorsey commented 5 years ago

Just now getting a chance to look over your factory work ;) I really love the idea of a test interface that echos back whatever you send it with a delay. That would be incredibly useful for a test suite.

I think I'm going to sit on both implementations and think about it for a bit. I've got a busy week coming up, but I'd love to bang my head against this stuff again next weekend. I think we should definitely aim for TCP support (and hopefully Windows as a result) by the Sec Shell workshop/talk on the 21st. You've really kicked me into gear with this, so thanks so much for your contributions.

I tested sending and receiving using your sockets enhancement and it looks like it works.

Yeah I was testing it a bit last night and it looks like it's groovy! The only thing it doesn't handle elegantly yet are failed and dropped connections, so I'll clean that up next weekend.

I think the way you want to go about integration testing this in a platform independent manner is to mock a TCP server.

Yeah that could definitely work, now that TCP is going to be supported. We could also borrow from your delayed echo idea and do that whenever the config value is"KissPort": "test://loopback" or something.

Anyway, I'll circle back next weekend. I'm definitely down to meet up and hack on some of this together ahead of the Sec Shell meetup. Although it would be nice if we could make remote contact too over the airwaves, ha.

danc256 commented 5 years ago

All sounds good. Mocking Dire Wolf can make it trivial to build a suite of failure cases to shore up your endpoints in failure cases. You can capture real communication between Chattervox and Dire Wolf with WireShark and deliberately tamper with the data to build realistic failures using a mock TNC server. Also capturing Dire Wolf’s audio files, corrupting those, and capturing the network traffic that results from corrupted audio might also be useful. Though I suspect Dire Wolf will insulate you from many of those cases by simply rejecting those corrupted transmissions. It’s a really powerful tool with far more options than I imagined.

I can put a little more time in on the Windows side. It really bugs me that the Microphone isn’t accessible. It might be one of those cases where a USB audio device would fix all the problems. Even if I get the microphone working there might still be an issue with impedance and the radio. Still, try the base case first. I hate buying crap if it’s not necessary.

LMK a day or two that looks good to get together. I can finally send a transmission with my shiny license and validate that effort.

From: Brannon Dorsey notifications@github.com Reply-To: brannondorsey/chattervox reply@reply.github.com Date: Sunday, March 10, 2019 at 10:24 PM To: brannondorsey/chattervox chattervox@noreply.github.com Cc: Dan Chisarick chisarickd@doombridge.com, Assign assign@noreply.github.com Subject: Re: [brannondorsey/chattervox] Add KISS support over TCP (#7)

Just now getting a chance to look over your factory work ;) I really love the idea of a test interface that echos back whatever you send it with a delay. That would be incredibly useful for a test suite.

I think I'm going to sit on both implementations and think about it for a bit. I've got a busy week coming up, but I'd love to bang my head against this stuff again next weekend. I think we should definitely aim for TCP support (and hopefully Windows as a result) by the Sec Shell workshop/talk on the 21st. You've really kicked me into gear with this, so thanks so much for your contributions.

I tested sending and receiving using your sockets enhancement and it looks like it works.

Yeah I was testing it a bit last night and it looks like it's groovy! The only thing it doesn't handle elegantly yet are failed and dropped connections, so I'll clean that up next weekend.

I think the way you want to go about integration testing this in a platform independent manner is to mock a TCP server.

Yeah that could definitely work, now that TCP is going to be supported. We could also borrow from your delayed echo idea and do that whenever the config value is"KissPort": "test://loopback" or something.

Anyway, I'll circle back next weekend. I'm definitely down to meet up and hack on some of this together ahead of the Sec Shell meetup. Although it would be nice if we could make remote contact too over the airwaves, ha.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/brannondorsey/chattervox/issues/7#issuecomment-471383290, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ARH_VOpk06di7RD4iPi2EEdgCnODHWnVks5vVb5TgaJpZM4W10na.

brannondorsey commented 5 years ago

Alrighty, I've banged my head against this stuff for a few more hours and I think I've got a release candidate that closes this issue. Here's the change I'm proposing: https://github.com/brannondorsey/chattervox/commit/9415a99f015e4b15fb94cddff7bacecfad8c11da.

I decided to go with the updated kiss-tnc library version of the solution instead of the new dependency + factory solution. In general, I think the factory is actually a pretty great idea! But this was a quick way to get what we wanted with minimal code change + without adding a new dependency. I really love your loopback idea though, so I'm thinking we can add that to this version too.

I'm gunna let it sit in the develop branch for a bit, and will likely merge to master tomorrow or this weekend.

danc256 commented 5 years ago

I think the advantages of TCP are significant and that TCP should be the default and the question should be “do you want to force serial instead”? There may be reasons someone wants to do serial, such as a real HW TNC or perhaps they have some filters or chains built using pipes. TCP is cross-platform, requires the least about of os-specific voodoo, and has lots of advantages over serial for diagnostics, testing, separating the chat client from the virtual TNC, etc. Also it will drive users to test the newer functionality but still be able to fallback to serial. TCP allows custom proxy servers so they can do things like stronger encryption transparently where laws allow such things and without modifying the code codebase.

One of the foremost general guidelines in contributing to a codebase is to try to stay as close to the existing development methodology as possible. It’s reasonable to reject the heavyweight changes I made. It still produced some new ideas (loopback interface for testing) and I got to do a little more with TypeScript and Node.js. I really think the loopback will be an embedded server that listens on localhost but it may also be easier for testing to have a custom URI that’s a special protocol just for testing.

From: Brannon Dorsey notifications@github.com Reply-To: brannondorsey/chattervox reply@reply.github.com Date: Wednesday, March 13, 2019 at 10:08 PM To: brannondorsey/chattervox chattervox@noreply.github.com Cc: Dan Chisarick chisarickd@doombridge.com, Assign assign@noreply.github.com Subject: Re: [brannondorsey/chattervox] Add KISS support over TCP (#7)

Alrighty, I've banged my head against this stuff for a few more hours and I think I've got a release candidate that closes this issue. Here's the change I'm proposing: 9415a99https://github.com/brannondorsey/chattervox/commit/9415a99f015e4b15fb94cddff7bacecfad8c11da.

I decided to go with the updated kiss-tnc library version of the solution instead of the new dependency + factory solution. In general, I think the factory is actually a pretty great idea! But this was a quick way to get what we wanted with minimal code change + without adding a new dependency. I really love your loopback idea though, so I'm thinking we can add that to this version too.

I'm gunna let it sit in the develop branch for a bit, and will likely merge to master tomorrow or this weekend.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHubhttps://github.com/brannondorsey/chattervox/issues/7#issuecomment-472676110, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ARH_VIue5q7AQ_4M71EAJHflIUMtgzm5ks5vWa8PgaJpZM4W10na.