SIGMA: Bugfixes in various device modules #356
Conversation
…ndalone System Exerciser to run. IO: DVT_NOTDEV macro incorrect, Device mapping algorithm creates false dispatch points. This mapped Multi Unit Controller and Single Unit Controller to same device. DP, DP, MT, RAD: Test for non-existent device returns wrong status. DP, DK, MT: TIO status should return non-operational for unattached device.
| 20-Jul-22 RMS Space record must set EOF flag on tape mark (Ken Rector) | ||
| 03-Jul-22 RMS Fixed error in handling of channel errors (Ken Rector) | ||
| 02-Jul-22 RMS Fixed bugs in multi-unit operation | ||
| 07-Jun-22 RMS Removed unused variables (V4) | ||
| 26-Mar-22 RMS Added extra case points for new MTSE definitions | ||
| 23-Mar-20 RMS Unload should call sim_tape_detach (Mark Pizzolato) |
There was a problem hiding this comment.
Is this only a comment change? Please confirm that the actual code change was in fact made before March 2022.
By the way, it would be more logical for changes to be marked with the initials of the person making the change, rather than by RMS, if Bob was not that person.
There was a problem hiding this comment.
What simulator is this for? This PR does not follow conventions.
There was a problem hiding this comment.
@pkoning2 - I looked back in the sigma_mt.c history and found that the unload/detach code change was made in a commit of files for several simulators by Mark P on April 4, 2020. Mark did not add a comment like this at that time.
I think Mark did not include these kinds of commemts, but Bob did in his classic simh archive. So, in my adding new code from Bob this comment slipped in. Mark did include comments of this type in later commits.
I could delete this comment or change its date to be more accurate or do nothing.
Your choice.
Bob did make these changes.
Usually I find a problem, do some research and give him a description of the problem and any ideas I have about how to fix it. Usually my fix works for me and Bob will consider that and come up with suitable code. I think he puts my name on it out of courtesy. If you don't like that, its not necessary by me, I have enough noteriety.
@rcornwell - Where could I find the PR conventions?
I'm eager to fix this. Can you get me started with how to do that.
Why did you both reference the sigma_mt.c changes?
Why did this fail two out of six build tests and what could I do to fix that?
There was a problem hiding this comment.
Ken,
Thanks for the confirmation. With that, I have no concern about the change. I mentioned sigma_mt.c specifically because that's where I saw the added comment referring to Mark, and that made me want to verify we didn't have a problem with the policy restricting changes imported from Mark's "simh" repository.
On the test failure, that's a machinery issue not related to your work. I don't know why this happens; it seems to be intermittent. You may ignore it.
|
@kenrector, For the record:
Historically, Bob has added one line comments like this in his simulators. In general, when dealing with his simulators I always more or less merged his code as provided by him, but sometimes I tried to isolate multiple changes he made to separate commits so that a comment would be with the related code change when I could determine that detail. Meanwhile, once we migrated to git, comments at the top of source code modules add pain rather than adding value. Git commit comments exist to explain the details of changes, and comments like these might be useful for the initial line of the commit message. The pain comes when multiple folks happen to be working on the same source modules and they make changes in the same areas. Normally, functional changes are absolutely never in the same areas, EXCEPT for the comments at the top of the source module. Such changes create the potential for automatic merge issues due to them being in the same place. The value of the commit comments tracking specifically with the actual relate changes exists with git automatically. additionally, @rcornwell said:
What he was specifically referring to was the format of the commit message (first line specifically). The PR request has a subject that follows the conventions that @rcornwell is referring to, BUT the actual commit message says: The convention generally is: |
Fix three kinds of error in device modules. Discovered while getting standalone System Exerciser to run.
IO: DVT_NOTDEV macro incorrect, Device mapping algorithm creates false dispatch points. This mapped Multi Unit Controller and Single Unit Controller to same device.
DP, DP, MT, RAD: Test for non-existent device returns wrong status.
DP, DK, MT: TIO status should return non-operational for unattached device.