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

Implement interrupt-driven UART transfers #267

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

twilkhoo
Copy link

@twilkhoo twilkhoo commented Feb 1, 2024

Purpose

https://www.notion.so/uworbital/Implement-interrupt-driven-UART-transfers-210f61abbeed4549bacdadde775f3bee?pvs=4
Make transfers from the RM46 to the IMU async (using UART).

New Changes

  • Changed function signature of sciSendBytes in obc_sci_io.
  • Added semaphore logic similar to sciReadBytes.
  • Enabled TX Transmit in HalCoGen.

Testing

  • Build test passes, other tests still pass.

Outstanding Changes

Copy link

github-actions bot commented Feb 1, 2024

Pull reviewers stats

Stats of the last 120 days for UWOrbital:

User Total reviews Time to review Total comments
Navtajh04 34 5d 16h 5m 286
dgobalak 32 5d 1h 43m 226
Gjjjiang 5 12d 17h 14m 16
Yarik-Popov 4 1d 2h 22m 60
manumanuk 2 1d 7h 24m 7
JoelManYunLee 1 17h 57m 4

obc/drivers/rm46/obc_sci_io.c Outdated Show resolved Hide resolved
@twilkhoo twilkhoo force-pushed the tejas/enable-async-uart-transfers branch from 9c04f4f to a465463 Compare February 24, 2024 04:08
@twilkhoo twilkhoo requested a review from Navtajh04 February 24, 2024 04:36
Copy link
Contributor

@Yarik-Popov Yarik-Popov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall seems good. Mainly add macros

@@ -391,7 +391,8 @@ static obc_error_code_t handleSendingConnState(void) {
}
obc_error_code_t errCode;
#if COMMS_PHY == COMMS_PHY_UART
RETURN_IF_ERROR_CODE(sciSendBytes(connCmdPkt.data, (uint32_t)connCmdPkt.length, portMAX_DELAY, UART_PRINT_REG));
RETURN_IF_ERROR_CODE(
sciSendBytes(connCmdPkt.data, (uint32_t)connCmdPkt.length, portMAX_DELAY, pdMS_TO_TICKS(100), UART_PRINT_REG));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add macro for pdMS_TO_TICKS(100) and change it everywhere this value is used. What does this represent? Why is it 100?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, changed to 1000ms to be more generous. It's just a value that should be enough for the transmit, and I'm now using 1000 to keep it in line with the timeout for reading the first byte (sciReadBytes in same file).

obc/drivers/vn100/vn100.c Outdated Show resolved Hide resolved
@twilkhoo twilkhoo requested a review from Yarik-Popov March 4, 2024 17:43
Copy link
Contributor

@Navtajh04 Navtajh04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be tested on board with vn100 before merge but lgtm other than that. Also make sure you saved the project after making the halcogen changes

obc/app/drivers/rm46/obc_sci_io.c Outdated Show resolved Hide resolved
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

Successfully merging this pull request may close these issues.

3 participants