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

No serial RX on UART1 when using non-default pins #401

Closed
PaulZC opened this issue Jun 5, 2021 · 16 comments
Closed

No serial RX on UART1 when using non-default pins #401

PaulZC opened this issue Jun 5, 2021 · 16 comments
Milestone

Comments

@PaulZC
Copy link
Contributor

PaulZC commented Jun 5, 2021

Background:

I'm updating OpenLog Artemis to use v2.1.0 of the core. All is going well(ish) but I'm not able to receive any serial data on UART1. OpenLog Artemis uses pin 12 for Tx and pin 13 for Rx. Serial Tx on pin 12 works fine, I'm just not able to receive anything on pin 13.

Steps to replicate:

RedBoard Artemis ATP
v2.1.0 of the core
Use a jumper wire to link pin 12 to pin 13
Run this sketch:

// Apollo3 v2.1.0 UART Loopback Test
// Board: RedBoard Artemis ATP
// Use a jumper wire to connect Pin 12 (TX) to Pin 13 (RX)

UART mySerial(12, 13);

void setup() {
  Serial.begin(115200);

  mySerial.begin(115200);

  Serial.println("UART Loopback Test");

  mySerial.println("Hey, it worked!");

  delay(100);

  while (mySerial.available())
  {
    Serial.write(mySerial.read());
  }  
}

void loop() {
}

Open the serial monitor, and you should see this (captured using Serial1 on pins 24 and 25):

image

Instead, we get this:

image

Putting a logic analyzer on the jumper wire, I can see pin 12 transmitting:

image

So the issue is somewhere in the receive functionality.

I started digging into the core UART code, but I haven't found a smoking gun so far. It looks like pin 13 should be being configured correctly:

https://github.com/sparkfun/mbed-os-ambiq-apollo3/blob/be07f057170a4e9ed4a286abb2170b9df3d52de3/targets/TARGET_Ambiq_Micro/TARGET_Apollo3/device/PeripheralPins.c#L104

https://github.com/sparkfun/mbed-os-ambiq-apollo3/blob/be07f057170a4e9ed4a286abb2170b9df3d52de3/targets/TARGET_Ambiq_Micro/TARGET_Apollo3/device/PeripheralPinConfigs.c#L500-L509

https://github.com/sparkfun/mbed-os-ambiq-apollo3/blob/be07f057170a4e9ed4a286abb2170b9df3d52de3/targets/TARGET_Ambiq_Micro/TARGET_Apollo3/sdk/mcu/apollo3/hal/am_hal_pin.h#L165

I'm out of ideas... Any suggestions on where to dig next?

Cheers!
Paul

@PaulZC
Copy link
Contributor Author

PaulZC commented Jun 6, 2021

More observations on this:

This works:

// Apollo3 v2.1.0 UART Loopback Test
// Board: RedBoard Artemis ATP
// Use a jumper wire to connect Pin 24 (TX) to Pin 25 (RX)

#define mySerial Serial1

image

This does not work:

// Apollo3 v2.1.0 UART Loopback Test
// Board: RedBoard Artemis ATP
// Use a jumper wire to connect Pin 24 (TX) to Pin 25 (RX)

UART mySerial(24, 25);

If I edit variant.cpp for the ARTEMIS_ATP and change this line:

UART Serial1(SERIAL1_TX, SERIAL1_RX);

to:

UART Serial1(D12, D13);

then this works:

// Apollo3 v2.1.0 UART Loopback Test
// Board: RedBoard Artemis ATP
// Use a jumper wire to connect Pin 12 (TX) to Pin 13 (RX)

#define mySerial Serial1

So, pin 12 and 13 do work OK. It's not a configuration or hardware issue as such. But something is stopping UART mySerial(12, 13); from working. I wonder if the core thinks UART1 is already in use (as Serial1) and so stops us redefining 'another' UART1?

If I edit variant.cpp for the ARTEMIS_ATP and comment line 8. And also edit variant.h and comment line 26:

Then my original script ( UART mySerial(12, 13); ) works fine on pins 12 and 13:

image

OK. So is there a way to persuade the core to let us 'reallocate' the pins for UART1 if Serial1 already exists? I shall dig some more...

@Wenn0101
Copy link
Contributor

Wenn0101 commented Jun 6, 2021

Hey Paul! interesting stuff so far!

I am trying out your sketches now. I'll let you know what I find!

@PaulZC
Copy link
Contributor Author

PaulZC commented Jun 6, 2021

There's no rush on this... (Please don't ruin your evening by diving into this!)

@PaulZC
Copy link
Contributor Author

PaulZC commented Jun 6, 2021

@Wenn0101
Copy link
Contributor

Wenn0101 commented Jun 6, 2021

I think you have really narrowed down the problem. The issue seems to be more about the fact that declaring a "new" serial should really be more changing the configuration of the already initialized hardware serial to new pins. I wonder if this logic should happen at a higher level.

@PaulZC
Copy link
Contributor Author

PaulZC commented Jun 7, 2021

More thoughts on this:

I can see how having two pins - 13 and 25 - configured for UART1 RX via the pin .uFuncSel would cause problems. But I don't think that is the core issue here. If it was then UART mySerial(24, 25); should work just fine; same UART, exact same pins... But it doesn't.

So, following the trail of breadcrumbs: PinMap_UART_RX is defined here. A pointer to PinMap_UART_RX is returned here in serial_rx_pinmap. serial_rx_pinmap is used in pinmap_config inside serial_init here. pinmap_config is defined here and calls am_hal_gpio_pinconfig via pinConfig here. am_hal_gpio_pinconfig does a lot but I don't see anything in there that would prevent the RX pin from being configured a second time. I don't think the issue is in there.

Going back to serial_init, the handle for UART1 will already exist - thanks to UART Serial1, so this piece of code will not get executed. Does that matter? Looking at am_hal_uart_initialize, I don't see anything in there to do with pins...

At the moment, this still has me stumped. I have a J-Link debug cable on order; I hope the debug interface may provide more clues...

A quick fix would be to define a new variant for the OLA, which does not define Serial1. But that wouldn't help anyone trying to use non-default pins on e.g. the ATP...

@PaulZC
Copy link
Contributor Author

PaulZC commented Jun 9, 2021

Is there any way that we could be calling gpio_mode accidentally when creating the new UART? gpio_mode calls ap3_hal_gpio_pinconfig_partial, but always sets .uFuncSel to AP3_PINCFG_FUNCSEL_GPIO. That would cause problems as we need .uFuncSel to be AM_HAL_PIN_13_UART1RX if we are to receive RX data on pin 13... Just a thought...

@Wenn0101 Wenn0101 added this to the v2.1.1 milestone Jun 10, 2021
@Wenn0101
Copy link
Contributor

Wenn0101 commented Jun 10, 2021

ok. Here is a spooky finding.

// Apollo3 v2.1.0 UART Loopback Test
// Board: RedBoard Artemis ATP
// Use a jumper wire to connect Pin 12 (TX) to Pin 13 (RX)

void setup() {
  UART mySerial(12, 13); //move the myserial constructor into set-up... a user wouldn't do this but for demonstration purposes....
  Serial.begin(115200);

  mySerial.begin(115200);

  Serial.println("UART Loopback Test");

  mySerial.println("Hey, it worked!");

  delay(100);

  while (mySerial.available())
  {
    Serial.write(mySerial.read());
  }  
}

void loop() {
}

This sketch works...
I think this may actually be a matter of which constructor is called first. Perhaps in your example the Serial1 object is being made after the mySerial object... I'll see if I can verify this.

@Wenn0101
Copy link
Contributor

Also, a workaround. I have fixed the Artemis Module target (on the dev branch not released yet), this may be a better suited base target for this kind of project.

@PaulZC
Copy link
Contributor Author

PaulZC commented Jun 10, 2021

Indeed - thank you! I did spot the issues with the "Module" but didn't want to spoil your day with them...
I think we're looking at an issue where reconfiguring a pin fails. And I suspect it is linked to Nate's Using-Pin-25-for-GPIO issue too.
I'm still waiting on my J-Link debug cable, now expected tomorrow. I'll share what I can as soon as that arrives.

@Wenn0101
Copy link
Contributor

I believe i fixed nates issue with this commit, but this could be related/similar.

@PaulZC
Copy link
Contributor Author

PaulZC commented Jun 12, 2021

So, it looks like the root cause of this might indeed be due to the order in which things happen - agreeing with your spooky finding.

If I upload this sketch:

UART mySerial(12,13);

void setup() {
}

void loop() {
}

and I have ATP selected as the board, then I see serial_init being called three times. The three times is probably expected: Serial, Serial1 and mySerial. But it is the order in which it happens that is causing the problem:

First call: note that R1 contains "0xC" = Pin 12 and that R2 contains "0xD" = Pin 13. I.e. this is the init for mySerial:

image

Second call: this must be Serial1 as R1 is 24 and R2 is 25:

image

Third call: this must be Serial (USB) as R1 is 48 and R2 is 49:

image

So, it looks like mySerial is being initialized correctly, first, but it is then being hijacked by Serial1! I don't yet have a theory on why I still see data being transmitted on Pin 12 though... Unless Pin 12 is left as a duplicate TX pin?

Time to test a jumper from pin 12 to pin 25... Watch this space!

@PaulZC
Copy link
Contributor Author

PaulZC commented Jun 12, 2021

Hah!

// Apollo3 v2.1.0 UART Loopback Test
// Board: RedBoard Artemis ATP
// Use a jumper wire to connect Pin 12 ( mySerial TX ) to Pin 25 ( Serial1 RX )

UART mySerial(12, 13);

void setup() {

  Serial.begin(115200);

  Serial1.begin(115200); // Note: both Serial1 and mySerial will attempt to use UART1!
  mySerial.begin(115200);

  Serial.println("UART Loopback Test");

  mySerial.println("Hey, it worked!");

  delay(100);

  while (Serial1.available())
  {
    Serial.write(Serial1.read());
  }  
}

void loop() {
}

image

@PaulZC
Copy link
Contributor Author

PaulZC commented Jun 12, 2021

So.... Serial and Serial1 are being serial_init'd even when they are not being used... Maybe we should fix that?!

@Wenn0101
Copy link
Contributor

I am going to close this one out.

The artemis module is now available to those creating new boards.
Serial and Serial1 are being inited so that they are available to all sketches without the need to do any extra set-up that is not "arduino-like". I don't know if there is a good way to re-declare the serial to have your own serial object, but I don't know if that is needed anymore. let me know if you think it needs a re-open.

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

2 participants