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

Add code to emulate a HIM device to connect MTS to the Internet #689

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

mtalexander
Copy link
Contributor

Emulate a Host Interface Machine (HIM) which is used to connect the Michigan Terminal System (MTS) to the Internet.

A HIM is a Host Interface Machine which provides an interfave between
the Michigan Terminal System (MTS) and the Internet.
@Fish-Git
Copy link
Member

THANK YOU, Mike! I will try to review your Pull Request and provide you with feedback as soon as possible,

@Fish-Git Fish-Git removed the request for review from ivan-w October 27, 2024 04:52
@Fish-Git
Copy link
Member

** FYI **     (mostly for myself):

Tom has asked me to cc him with whatever feedback I provide to Mike:

-----Original Message-----
From: Thomas Valerio [mailto:[email protected]]
Sent: Sunday, October 27, 2024 10:53 AM
To: "Fish" (David B. Trout)
Cc: Mike Alexander
Subject: Hercules - Multi-platform MTS/HIM device
Importance: High

Fish,
I don't know if Mike communicated with you directly regarding his
HIM device pull request, but he will be traveling from next Monday
through November 17, and will probably have no or minimal Internet
access and usually generally ignores his email when traveling anyway.

So if you have any direct comment or criticism of the code before his
return could you please cc me on any email that you might send directly
to Mike.

-- Tom/

@Fish-Git
Copy link
Member

Fish-Git commented Nov 16, 2024

Sorry for the delay. I haven't forgotten about you! I'm just having some serious personal issues right now.

@tjvalerio
Copy link

Any comment at all so far on this pull request and/or a guess on the ETM (expected time to merge)?

@Fish-Git
Copy link
Member

Fish-Git commented Dec 16, 2024

@tjvalerio

Any comment at all so far on this pull request and/or a guess on the ETM (expected time to merge)?

My apologies. Things were crazy at my household for this past month or more:

I'm only now trying to get back into the swing of things.

I'm looking at your Pull Request right now. It looks like it's going to need a few very minor changes.

One thing that would make it easier on me would be if you could pull/merge all of the latest develop branch changes that have been made since you first created your pull request into your HIM branch. That way I could more easily see only your HIM changes by comparing your HIM branch against the current develop branch. Do you think you could do that?

That is to say, once you do a clone of Hercules and begin to make your HIM changes, you still need to periodically do a 'git pull' from the develop branch to pick up all of the recent changes that the rest of the Hercules developers having been doing. You still need to keep your repository "current".

I've only spent the past few minutes reviewing your changes and so far I can confidently say this about them: they look fine to me so far!  :)

But give me another day (or two? or three?) and I'll post a more formal review for you.

Fair?

Copy link
Member

@Fish-Git Fish-Git left a comment

Choose a reason for hiding this comment

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

(CC: @tjvalerio Tom)

The only required change is the one to the UDP_sserver_listen_thread function: lines 1823-1826 need to be moved to the beginning of the function. (I am using an older 'C' compiler and insist that it still be supported. Once I'm dead the other surviving members of the Hercules team/project can repeal that requirement as they wish, but as long as I'm alive, I'm going to insist that that requirement remain. Sorry!)

ALL OTHER CHANGES are purely cosmetic and optional. I removed a few blank lines here and there, moved a few opening braces on some if and for statements to be on their own separate line, inserted some opening/closing braces where some locks where being obtained/released to better see the lock's scope, etc, but that's about it! Everything else looked fine!

  • MTS.patch     (required and suggested changes)

Just make that one minor required change, and I would be more than happy to merge your changes into Hercules! I think having MTS HIM device support would be an excellent addition to Hercules!

THANK YOU for your effort, Mike!  :)

I look forward to seeing an updated Pull Request!  :)

@mtalexander
Copy link
Contributor Author

mtalexander commented Dec 22, 2024 via email

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