Skip to content

Conversation

@QuackWifHat
Copy link
Contributor

Payload integration with main

@QuackWifHat QuackWifHat requested a review from devYaoYH May 20, 2025 03:59
Copy link
Contributor

@devYaoYH devYaoYH left a comment

Choose a reason for hiding this comment

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

Run linter using ./format_all.sh from repo root or src/.

@QuackWifHat
Copy link
Contributor Author

Ran the ./format_all.sh, but when I ran git status it looks like there's nothing to really push. Not sure I'm understanding how to use the linter right

@devYaoYH
Copy link
Contributor

Looking into the logs for the failing lint check:

Posting comment(s)
  Notice: File src/slate/slate.h does not conform to Custom style guidelines. (lines 84, 85)
  Notice: File src/states/running/running_state.c does not conform to Custom style guidelines. (lines 17, 18, 19, 20)
  Notice: File src/states/running/running_state.h does not conform to Custom style guidelines. (lines 7)
  INFO:CPP Linter:3 clang-format-checks-failed
  INFO:CPP Linter:0 clang-tidy-checks-failed
  INFO:CPP Linter:3 checks-failed

So those lines above have formatting that doesn't conform to our style guides. ./format_all.sh should run and modify these files in-place, so you should see 3 files changing afterwards.

Also make sure to pull this branch again (since I did the merge using the visual editor online - this commit won't be on your local).

@devYaoYH
Copy link
Contributor

devYaoYH commented May 20, 2025

Also please fix the failing builds for PICO-type platform builds.

You should be able to mock out these PICUBED pins:

/home/runner/work/samwise-flight-software/samwise-flight-software/src/drivers/payload_uart/payload_uart.c: In function 'payload_turn_on':
/home/runner/work/samwise-flight-software/samwise-flight-software/src/drivers/payload_uart/payload_uart.c:191:14: error: 'SAMWISE_RPI_ENAB' undeclared (first use in this function); did you mean 'SAMWISE_RF_SPI'?
  191 |     gpio_put(SAMWISE_RPI_ENAB, 1);
      |              ^~~~~~~~~~~~~~~~
      |              SAMWISE_RF_SPI
/home/runner/work/samwise-flight-software/samwise-flight-software/src/drivers/payload_uart/payload_uart.c:191:14: note: each undeclared identifier is reported only once for each function it appears in
/home/runner/work/samwise-flight-software/samwise-flight-software/src/drivers/payload_uart/payload_uart.c: In function 'payload_turn_off':
/home/runner/work/samwise-flight-software/samwise-flight-software/src/drivers/payload_uart/payload_uart.c:196:14: error: 'SAMWISE_RPI_ENAB' undeclared (first use in this function); did you mean 'SAMWISE_RF_SPI'?
  196 |     gpio_put(SAMWISE_RPI_ENAB, 0);
      |              ^~~~~~~~~~~~~~~~
      |              SAMWISE_RF_SPI
/home/runner/work/samwise-flight-software/samwise-flight-software/src/drivers/payload_uart/payload_uart.c: In function 'payload_uart_init':
/home/runner/work/samwise-flight-software/samwise-flight-software/src/drivers/payload_uart/payload_uart.c:208:23: error: 'SAMWISE_UART_TX' undeclared (first use in this function)
  208 |     gpio_set_function(SAMWISE_UART_TX,
      |                       ^~~~~~~~~~~~~~~
[ 58%] Building C object src/drivers/payload_uart/CMakeFiles/payload_uart.dir/__/__/__/pico-sdk/src/rp2_common/hardware_sync/sync.c.o
/home/runner/work/samwise-flight-software/samwise-flight-software/src/drivers/payload_uart/payload_uart.c:210:23: error: 'SAMWISE_UART_RX' undeclared (first use in this function)
  210 |     gpio_set_function(SAMWISE_UART_RX,
      |                       ^~~~~~~~~~~~~~~
/home/runner/work/samwise-flight-software/samwise-flight-software/src/drivers/payload_uart/payload_uart.c:214:15: error: 'SAMWISE_RPI_ENAB' undeclared (first use in this function); did you mean 'SAMWISE_RF_SPI'?
  214 |     gpio_init(SAMWISE_RPI_ENAB);
      |               ^~~~~~~~~~~~~~~~
      |               SAMWISE_RF_SPI

You can define these for non-PICO builds only with:

#ifndef PICO
#define SAMWISE_UART_RX // as per normal pinout on PICUBED
#else
#define SAMWISE_UART_RX // as per mocked pins on PICO (pick something not currently in use)
#endif

A successful build check like here should pass for all our configured platform profiles.

Copy link
Contributor

@devYaoYH devYaoYH left a comment

Choose a reason for hiding this comment

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

PTAL at the comments, some are forward-looking (and you don't have to address them in this PR), but primarily clean up some naming + (what I think might've been) spurious changes to the radio_task.

src/main.c Outdated
/*
* Initialize everything.
*/
sleep_ms(5000);
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks new... we probably don't need the explicit sleep here.

Copy link
Contributor

@devYaoYH devYaoYH left a comment

Choose a reason for hiding this comment

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

Just a couple more minor edits necessary:

  1. remove extra 5s sleep in main.c
  2. add rfm9x_transmit(&slate->radio); in radio_task dispatch call when tx queue is not empty (sets radio to transmission mode)

Copy link
Contributor

@devYaoYH devYaoYH left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! LGTM

@QuackWifHat QuackWifHat merged commit 110e1fe into main May 24, 2025
3 checks passed
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