NRF52_DK Serial RX doesn't work

[pan-]

Description

The NRF52_DK serial is not capable of receiving bytes:

int main() {
    RawSerial serial(USBTX, USBRX);

    serial.printf("Hello world\r\n");

    while (true) {
        if (serial.readable()) {
            serial.putc(serial.getc());
        }
    }
}

The trace Hello world is printed correctly but the device does not capture any byte send to it whether the serial is polled or operating under interrupts.

The same application worked prior to 3663494 .

Issue request type

[ ] Question
[ ] Enhancement
[X] Bug

[marcuschangarm]

You are creating a new serial object so by default flow control is disabled. You'll have to enable it explicitly:

serial.set_flow_control(SerialBase::RTSCTS, RTS_PIN, CTS_PIN);

Alternatively, you should consider using the new file handles for reading and writing to STDIN/STDOUT.

[Mirrored to Jira]

[pan-]

I'm not sure to understand your answer, are you proposing a short term workaround or are you stating that backward compatibility has been broken and it will stays that way. Could you clarify this point ?
[Mirrored to Jira]

[marcuschangarm]

I'm saying a couple of things: 😄

1. The code above shouldn't have worked. The NRF52_DK DAPLINK requires flow control to be enabled.
2. The code above only worked because of a hack that has now been removed.
3. This, along with a lot of breaking changes are scheduled for 5.9.
4. You should consider using the new file handlers instead of RawSerial.

[Mirrored to Jira]

[donatieng]

Thanks for the clarification @marcuschangarm.

However it does not seem to work with DAPLink 0246 + flow control enabled with RawSerial (on the NRF52840-PDK at least).

@c1728p9 @theotherjimmy do you guys know why flow control needs to be enabled on DAPLink for these targets? It's not the case on JLink for instance which detects whether flow control is in use by sampling the state of the RTS pin at startup.

I think that before this lands to 5.9 we need to document these changes very clearly to our users and look at the impact on existing teams/tooling/etc before making this transition.

@bulislaw what's the current HAL policy for these kind of changes?
[Mirrored to Jira]

[marcuschangarm]

I've rerun the code below on both the NRF52_DK and NRF52840_DK with DAPLINK 246 on both and it works here.

int main(void)
{
    RawSerial serial(P0_6, P0_8, 115200);
    serial.set_flow_control(SerialBase::RTSCTS, P0_5, P0_7);
    serial.printf("hello world\r\n");

    while (true) {
        if (serial.readable()) {
            serial.putc(serial.getc());
        }
    }
}

I wonder if we are on different chip revisions? I have:

N52840
Q1AAAA
1716AK

N52832
QFAAB0
1610AK

[Mirrored to Jira]

[bulislaw]

We'll be rewriting the UART HAL very soon, if you have any hints or requests for HAL spec drop me an email.
[Mirrored to Jira]

[pan-]

@bulislaw @0xc0170 Is it acceptable to break backward compatibility and create code incompatibilities between boards such as basic code as the one I exposed above won't work ? To me it seems to break the mbed promise that the platform abstract away board differences and the same code application code can be executed if the boards support the feature.

@marcuschangarm Would it be possible to disable the hack that enable flow control using a macro instead of going the hard way and document all this ?
[Mirrored to Jira]

[0xc0170]

Reviewing the thread, and this one:

The code above shouldn't have worked. The NRF52_DK DAPLINK requires flow control to be enabled.

I find the above code as valid and should work (nordic would enable flow control by default to have it working). Shouldn't this be the case?
[Mirrored to Jira]

[marcuschangarm]

I find the above code as valid and should work (nordic would enable flow control by default to have it working). Shouldn't this be the case?

You are creating a new serial object with a set of pins, presumably connected to a serial device. Why would flow control be enabled by default? What pins would be used for flow control? What other board magically enables flow control behind your back and sets it to two arbitrary pins?

We have paying customers utterly confused why flow control behaves inconsistently from the documentation.
[Mirrored to Jira]

[marcuschangarm]

Is it acceptable to break backward compatibility and create code incompatibilities between boards such as basic code as the one I exposed above won't work ? To me it seems to break the mbed promise that the platform abstract away board differences and the same code application code can be executed if the boards support the feature.

We are abstracting away differences. If you use the default stdout, either with printf or filehandle read/write it works out of the box across all platforms.

You are explicitly creating a new serial object from scratch. It would be an anti-pattern if flow control was enabled automatically on some boards and not others, and with arbitrary pins.
[Mirrored to Jira]

[pan-]

You are explicitly creating a new serial object from scratch. It would be an anti-pattern if flow control was enabled automatically on some boards and not others, and with arbitrary pins.

Why would these pins be arbitrary ? I though they were the CTS and RTS pins connected to chip running the DAPLink firmware.

You are explicitly creating a new serial object from scratch.

I think the issue lies here, we do not have access to an already built serial instance that handles stdin and stdout. Everything has to be built from scratch and I do agree that random flow control is confusing in that case however, the situation would be very different for a prebuilt instance of a peripheral configured to achieve a specific task (TX and RX in this case).

[Mirrored to Jira]

[marcuschangarm]

Why would these pins be arbitrary ? I though they were the CTS and RTS pins connected to chip running the DAPLink firmware.

That's because from your point of view RawSerial means DAPLink connection. With the old implementation, RawSerial(wifi_tx, wifi_rx) would still enable flow control on daplink_cts and daplink_rts, which is really confusing as a user.

For reference, this is how it looks like in Greentea: https://github.com/ARMmbed/mbed-os/blob/master/features/frameworks/greentea-client/source/greentea_serial.cpp

we do not have access to an already built serial instance that handles stdin and stdout.

Why can't you use the FileHandle API for reading and writing instead? That would solve all your problems across all platforms regardless of DAPLINK.

[Mirrored to Jira]