heath/h19.cpp Implement Shift-Reset functionality #11330
Conversation
|
This has way too much wrong with it. |
| m_drq_allowed = false; | ||
| m_access_track_sector = false; | ||
|
|
||
| m_fdc->reset(); |
There was a problem hiding this comment.
Child devices are reset automatically on reset – you shouldn’t be resetting them from device_reset.
| m_crtc->reset(); | ||
| m_ace->reset(); | ||
| m_beep->reset(); | ||
| m_mm5740->reset(); | ||
| m_maincpu->reset(); |
There was a problem hiding this comment.
Once again, you shouldn’t be resetting child devices from device_reset.
| void heath_tlb_device::mm5740_data_ready_w(int state) | ||
| { | ||
| check_for_reset(); | ||
|
|
There was a problem hiding this comment.
Is this when the reset key combination is actually checked, or does it happen out-of-band? If it happens out-of-band, you should attach callbacks to the relevant keys so you get a notification when they’re changed.
| void heath_intr_cntrl::device_reset() | ||
| { | ||
| m_intr_lines = 0; | ||
| } | ||
|
|
There was a problem hiding this comment.
This value comes from inputs. You shouldn’t change it on reset. If the devices that produce those inputs deassert their outputs on reset, it’s their responsibility to update the outputs on reset.
| m_h37->reset(); | ||
| m_intr_cntrl->reset(); | ||
| m_console->reset(); | ||
| m_serial1->reset(); | ||
| m_serial2->reset(); | ||
| m_serial3->reset(); | ||
| m_maincpu->reset(); |
There was a problem hiding this comment.
Once again, you shouldn’t be resetting child devices here – that happens automatically.
| void h89_state::reset_line(uint8_t data) | ||
| { | ||
| if (data == ASSERT_LINE) | ||
| { | ||
| reset(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Read/write line members conventionally use int values.
| m_interrupts_blocked = false; | ||
| m_drq_raised = false; | ||
| m_fd_irq_raised = false; |
There was a problem hiding this comment.
You shouldn’t be changing inputs on reset. If the devices connected to those inputs deassert them on reset, it’s their responsibility to push the change.
This reverts commit 2c02460. This PR obviously wasn't reviewed properly. It has very obvious issues, in particular: * Resetting child devices from device_reset/machine_reset. Child devices are automatically reset when a device is reset. * Changing input state on reset. This leads to state getting out of sync. Devices that change their outputs on reset are responsible for pushing out the update.
|
FYI to do a full system reset: |
But that will also reset things attached to serial ports, etc. Calling |
|
Reverting seems a little excessive when the machine runs fine with the new functionality and there is existing code that does the same things that this PR does wrong in such popular systems as C64/C128- https://github.com/mamedev/mame/blob/master/src/mame/commodore/c64.cpp#L1447 (and there are may other examples of it in the code). Anyways, new PR #11339 addresses the comments raised in this PR. |
)" This reverts commit 2c02460. This PR obviously wasn't reviewed properly. It has very obvious issues, in particular: * Resetting child devices from device_reset/machine_reset. Child devices are automatically reset when a device is reset. * Changing input state on reset. This leads to state getting out of sync. Devices that change their outputs on reset are responsible for pushing out the update.
)" This reverts commit 2c02460. This PR obviously wasn't reviewed properly. It has very obvious issues, in particular: * Resetting child devices from device_reset/machine_reset. Child devices are automatically reset when a device is reset. * Changing input state on reset. This leads to state getting out of sync. Devices that change their outputs on reset are responsible for pushing out the update.
The H19 allowed the user to press the right-shift and reset key to reset the system. Since the h89 shared the common terminal board in the H19, this reset also was routed to the h89 cpu board and reset the entire computer. The actual reset occurs on the release of the keys.
In mame, the default key for reset is F10. Using the right-shift and F10 now causes the machine (either H19 or H89) to be reset.
A few other general cleanup of comments, formatting, etc. is also done in this PR.