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

UART0 full-duplex CTS/RTS #125

Open
S0urceror opened this issue Dec 10, 2023 · 8 comments
Open

UART0 full-duplex CTS/RTS #125

S0urceror opened this issue Dec 10, 2023 · 8 comments

Comments

@S0urceror
Copy link

The current version does not support full-duplex CTS/RTS. It does support RTS (vdp) and CTS (mos) but not vice-versa. So fast transfers from EZ80 to ESP32 are guarded but the other way around not. If you want to send a lot of data from ESP32 to EZ80 you get buffer overruns quickly.

Relevant code in VDP:

void setupVDPProtocol() {
	VDPSerial.end();
	VDPSerial.setRxBufferSize(UART_RX_SIZE);					// Can't be called when running
	VDPSerial.begin(UART_BR, SERIAL_8N1, UART_RX, UART_TX);
	VDPSerial.setHwFlowCtrlMode(HW_FLOWCTRL_RTS, 64);			// Can be called whenever
	VDPSerial.setPins(UART_NA, UART_NA, UART_CTS, UART_RTS);	// Must be called after begin
}

Should be:

void setupVDPProtocol() {
	VDPSerial.end();
	VDPSerial.setRxBufferSize(UART_RX_SIZE);					// Can't be called when running
	VDPSerial.begin(UART_BR, SERIAL_8N1, UART_RX, UART_TX);
	VDPSerial.setHwFlowCtrlMode(HW_FLOWCTRL_CTS_RTS, 64);			// Can be called whenever
	VDPSerial.setPins(UART_NA, UART_NA, UART_CTS, UART_RTS);	// Must be called after begin
}

Relevant code in MOS:

	UCHAR	pins = PORTPIN_ZERO | PORTPIN_ONE;						// The transmit and receive pins											
	serialFlags &= 0xF0;
	SETREG(PD_DDR, pins);											// Set Port D bits 0, 1 (TX. RX) for alternate function.
	RESETREG(PD_ALT1, pins);
	SETREG(PD_ALT2, pins);
	if(pUART->flowControl == FCTL_HW) {
		SETREG(PD_DDR, PORTPIN_THREE);								// Set Port D bit 3 (CTS) for input
		RESETREG(PD_ALT1, PORTPIN_THREE);
		RESETREG(PD_ALT2, PORTPIN_THREE);
		serialFlags |= 0x02;
	}
        ...
        UART0_MCTL = 0x00;												// Bring modem control register to reset value

And:

UART0_wait_CTS:		GET_GPIO	PD_DR, 8		; Check Port D, bit 3 (CTS)
			JR		NZ, UART0_wait_CTS
			RET

Should be:

	UCHAR	pins = PORTPIN_ZERO | PORTPIN_ONE;						// The transmit and receive pins											
	serialFlags &= 0xF0;
	SETREG(PD_DDR, pins);											// Set Port D bits 0, 1 (TX. RX) for alternate function.
	RESETREG(PD_ALT1, pins);
	SETREG(PD_ALT2, pins);
	if(pUART->flowControl == FCTL_HW) {
		SETREG(PD_DDR, PORTPIN_THREE|PORTPIN_FOUR);				// Set Port D bit 3, 4 (CTS, RTS) for alternate function
		RESETREG(PD_ALT1, PORTPIN_THREE|PORTPIN_FOUR);
		SETREG(PD_ALT2, PORTPIN_THREE|PORTPIN_FOUR);
		serialFlags |= 0x02;
	}
        ...
        UART0_MCTL = 0x02;												// Enable RTS

And:

UART0_wait_CTS:		in0 a, (UART0_MSR)
			bit 4,a ; check inverted CTS bit, 1 = CTS, 0 = NOT CTS (clear to send)
			JR		Z, UART0_wait_CTS
			RET
@stevesims
Copy link
Contributor

this is cool stuff, and should be useful, especially when we add in facilities to send more data back from the VDP to the eZ80

one quick question - and forgive my ignorance of comms things... this is a pair of PRs, so does that mean that both are needed to be implemented simultaneously?

if so then that could be problematic for upgrading - feels like it means that both MOS and VDP need to be updated at the same time for this to work, which may not be practical

perhaps this may mean that we need to have a command to enable proper full-duplex comms?

@S0urceror
Copy link
Author

Yes, you are right we have to be careful in upgrading. Because both sides have to work full-duplex to make it work.

We can however do a graceful fallback. The VDUStreamProcessor::wait_eZ80() is waiting for the EZ80. We could extend it to go back to half-duplex if there is not a good handshake with MOS within a certain time-interval. Similar to how it goes back to a lower baudrate if things don't work as expected.

I can have a look at that if you think this is the direction to go. This would allow to upgrade VDP without a MOS upgrade. The other way around to do a MOS upgrade without VDP would require changes on the MOS side.

@S0urceror
Copy link
Author

Okay, checked things:

  • VDP hw-cts-rts + MOS hw-cts (default) => locks up in PACKET_GP echo in VDP because CTS is floating high
  • VDP hw-rts (default) + MOS hw-cts-rts => locks up in PACKET_GP send in MOS because RTS is floating high

So you cannot have new VDP with default MOS and vice-versa.

Proposed solution direction:

  • assume people have still default MOS
  • install new VDP hw-cts-rts
  • this will start with hw-rts serial (default)
  • if new PACKET_SERIAL is first received in vdu-sys, reinitialise hw-cts-rts serial
  • else PACKET_GP is received and echo is send in the correct mode
  • continue

This allows people to first upgrade VDP and experience no problems. Then continue to flash a new MOS and continue without problems.

Let me know what you think of this. I'll make the necessary changes to VDP and test things with both the default MOS and the hw-cts-rts MOS.

@stevesims
Copy link
Contributor

so as it goes, the "general poll" message that MOS sends to the VDP has one byte which gets sent as part of the handshake, which the VDP then echos back as part of the PACKET_GP message

up until right now, all versions of MOS send a 1 byte in that VDU command (it's aVDU 23,0,&80,1)

so what we could do instead is use a different sequence, and a different response

MOS could send a 2 byte instead. VDP could detect that and if initialised == false instead of just echoing it back respond with a 3 to indicate it supports hw-rts-cts. Once the 3 response has been sent, VDP should swap to hw-rts-cts, and when the 3 response is received MOS should also swap.

when initialised == true VDP would revert to doing its current simple echo for the "general poll" command - not that anything is really likely to be using it, but you never know

(if VDP receives a 1 then you know you have an old MOS that won't do hw-rts-cts)

this way there's no need to add in a new VDP protocol packet - we just use what's already there

@S0urceror
Copy link
Author

:-) that was initially what I was thinking as well before settling on a separate packet. I'll give it some thought and create a proof of concept and let you know.

@S0urceror
Copy link
Author

Made a couple of changes and tested things.

If you have the full-duplex capable VDP it will first start half-duplex and wait for the PACKET_GP. If PACKET_GP=1 this signals a half-duplex MOS. If PACKET_GP>1 this signals full-duplex. The full-duplex capable vdp will then switch to full-duplex mode and answer with a normal echo packet.

Since most people use the flash tool this assume they first upgrade VDP before upgrading MOS. And this allows for a clean upgrade path. I tested this and this works beautifully.

@TurboVega
Copy link

Is PORTPIN_FOUR the correct pin? The schematic shows DTR0 on that pin, not RTS0, which is on PORTPIN_TWO.

@TurboVega
Copy link

TurboVega commented Mar 14, 2024

The changes that I have for BDPP on MOS and VDP include full duplex RTS and CTS, but the EZ80 RTS and ESP32 CTS are not setup until BDPP is enabled. The startup is using ESP32 RTS to EZ80 CTS only, albeit with several fixes to MOS, because it was using the wrong register and testing the value incorrectly, as was also discovered by @S0urceror, looking at the code changes that he posted above. Since BDPP also uses the initial version exchange to know what to do, we can modify that code to handle setting up the EZ80 RTS to ESP32 CTS, based on the protocol versions, even though BDPP is initially disabled on startup. Thus, I suggest that we just fix full duplex by merging the BDPP changes, after adding the mentioned addition, rather than fixing FD separately. Doing so may make any existing merge request based on this particular issue ticket moot.

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

No branches or pull requests

3 participants