Skip to content

Add comma body firmware#2291

Merged
adeebshihadeh merged 13 commits intomasterfrom
body
Oct 25, 2025
Merged

Add comma body firmware#2291
adeebshihadeh merged 13 commits intomasterfrom
body

Conversation

@jhuang2006
Copy link
Copy Markdown
Contributor

No description provided.

@jhuang2006 jhuang2006 force-pushed the body branch 8 times, most recently from 6aec3bf to 47caf95 Compare October 15, 2025 21:51
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you shouldn't need this one. this is for pandas

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same with this file

// M1 drive: PB8 -> TIM16_CH1, PB9 -> TIM17_CH1, PE2/PE3 enables
// M2 drive: PA2 -> TIM15_CH1, PA3 -> TIM15_CH2, PE8/PE9 enables

#ifndef MOTOR_PWM_TIMER_CLOCK_HZ
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what's up with all the ifndef??

#define MOTOR_SPEED_CONTROL_UPDATE_PERIOD_US 1000U
#endif

#define MOTOR_CONTROL_UNUSED __attribute__((unused))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what's this for?

@jhuang2006 jhuang2006 marked this pull request as draft October 16, 2025 01:37
Copy link
Copy Markdown
Contributor

@adeebshihadeh adeebshihadeh left a comment

Choose a reason for hiding this comment

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

Let's clean this up and get it merged as clean base FW that just runs on the body board, then we can add the rest of the functionality in smaller followup PRs.

Comment on lines +11 to +15
void board_body_init_bootloader(void) {
}

void board_body_tick(void) {
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove all these things that are unused. the board struct isn't the same as panda's so you don't have to populate all these unused fields

Comment on lines +25 to +27
// Body board: PD0 (RX), PD1 (TX)
set_gpio_pullup(GPIOD, 0, PULL_NONE);
set_gpio_alternate(GPIOD, 0, GPIO_AF9_FDCAN1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

wrong indentation

Comment on lines +34 to +40
uint32_t board_body_read_voltage_mV(void) {
return 0U;
}

uint32_t board_body_read_current_mA(void) {
return 0U;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove these too


// ******************* Definitions ********************
#define HW_TYPE_UNKNOWN 0U
#define HW_TYPE_BODY 0xB0U
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i like the b prefix. let's call it 0xb1 or 0xb2 though?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we should pull out all these common ones between panda, jungle, and body. but we can do that in a followup

Comment on lines +44 to +49
// Body board: PD0 (RX), PD1 (TX)
set_gpio_pullup(GPIOD, 0, PULL_NONE);
set_gpio_alternate(GPIOD, 0, GPIO_AF9_FDCAN1);

set_gpio_pullup(GPIOD, 1, PULL_NONE);
set_gpio_alternate(GPIOD, 1, GPIO_AF9_FDCAN1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you do this in the board init?

@jhuang2006 jhuang2006 marked this pull request as ready for review October 21, 2025 08:19
Comment on lines +17 to +18
void board_body_set_can_mode(uint8_t mode) {
if (mode == CAN_MODE_NORMAL) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

doesn't need (or have) modes

# ****************** Motor Control *****************

def motor_set_speed(self, motor: int, speed: int) -> None:
assert motor in (1, 2), "motor must be 1 or 2"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we should make nice enums for the motors. like which one is 1 and 2?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this should go up one level. the boards/ folder is for different body motherboards

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here

const uint8_t led_pin[3];
const uint8_t led_pwm_channels[3]; // leave at 0 to disable PWM
board_init init;
board_init_bootloader init_bootloader;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is unused, along with a bunch of other stuff

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

compile errors if we remove init_bootloader and led driver requires pwm channels to be declared. removed all other unused

@adeebshihadeh
Copy link
Copy Markdown
Contributor

@rexblade21 This is pretty close, but there's still a decent amount to trim here. Before requesting another review, I recommend talking to an LLM about the PR, e.g. "Do you see any way we can simplify parts of this diff?"

Comment on lines +24 to +25
.led_GPIO = {GPIOC, GPIOC, GPIOC},
.led_pin = {7, 7, 7},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is confusing but i assume done to maintain compatibility with panda? if so, leave a comment

also, do we only have a single color LED?

Copy link
Copy Markdown
Contributor Author

@jhuang2006 jhuang2006 Oct 22, 2025

Choose a reason for hiding this comment

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

yes to maintain compatibility with led driver. and yes
given that its just a single blinking heartbeat led, we don't really have to use the led driver?

.led_pin = {7, 7, 7},
.init = board_body_init,
.enable_can_transceiver = board_body_enable_can_transceiver,
.has_spi = false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

don't need this


// ******************** Prototypes ********************
typedef void (*board_init)(void);
typedef void (*board_init_bootloader)(void);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unused

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unused

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

./board/early_init.h: In function 'early_initialization':
./board/early_init.h:56:3: error: implicit declaration of function 'detect_board_type' [-Werror=implicit-function-declaration]
56 | detect_board_type();

seems to be required for early_init. moving to board_body.h

board/body/can.h Outdated
Comment on lines +32 to +34
if (!body_can_bus_ready(bus)) {
return false;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

does this matter? don't we just recover anyway? or is this to prevent bus off such that we don't need to recover?


safety_rx_invalid += safety_rx_hook(&to_push) ? 0U : 1U;
#ifdef PANDA_BODY
body_can_safety_rx(&to_push);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this should just be safety_rx_hook

@adeebshihadeh
Copy link
Copy Markdown
Contributor

You need to ignore the body for the MISRA mutation tests: https://github.com/commaai/panda/actions/runs/18775663943/job/53570424244?pr=2291

@adeebshihadeh adeebshihadeh changed the title Body Refactor Add comma body firmware Oct 25, 2025
@adeebshihadeh adeebshihadeh merged commit 515ac45 into master Oct 25, 2025
6 checks passed
@adeebshihadeh adeebshihadeh deleted the body branch October 25, 2025 00:14
heysenbug pushed a commit to Ever-Cars/panda that referenced this pull request Feb 15, 2026
* motors

* can

* cleanup unused stuff

* initial clean

* more clean

* remove integral and derivative clamps, revert pwm driver to original

* remove integral and derivative clamps, revert pwm driver to original

* remove integral and derivative clamps, revert pwm driver to original

* dont need this for now

* clean

* fix can rx and can version error

* ignore body for misra mutation test

* fix bus recovery, remove body rx hook
heysenbug added a commit to Ever-Cars/panda that referenced this pull request Feb 15, 2026
* spi debug (commaai#2292)

Co-authored-by: Comma Device <device@comma.ai>

* Add comma body firmware (commaai#2291)

* motors

* can

* cleanup unused stuff

* initial clean

* more clean

* remove integral and derivative clamps, revert pwm driver to original

* remove integral and derivative clamps, revert pwm driver to original

* remove integral and derivative clamps, revert pwm driver to original

* dont need this for now

* clean

* fix can rx and can version error

* ignore body for misra mutation test

* fix bus recovery, remove body rx hook

* SocketPanda improvements (commaai#2297)

* SocketPanda improvements

* implement timeouts

* Fix mcu_type for deprecated pandas (commaai#2296)

* skip 1024 samples to settle, around 22ms (commaai#2295)

* skip 1024 samples to settle, around 22ms

* smaller diff

* Fix mcu_type in jungle (commaai#2300)

should fix jungle

* Cuatro siren (commaai#2294)

* Fix siren

* fix MISRA

* remove MISRA suppression

* disable amp

* fix rebase mistake

* fix fault

* diff audio

* misra

---------

Co-authored-by: Comma Device <device@comma.ai>

* add double buffer for microphone (commaai#2299)

* add double buffer for microphone

* comment

* fix size check

* Revert `mcu_type` changes (commaai#2303)

* Revert "Fix mcu_type for deprecated pandas (commaai#2296)"

This reverts commit 6c9064c.

* Revert "Fix mcu_type in jungle (commaai#2300)"

This reverts commit 1e8fa51.

* it's just unsupported

* cleanup fan scripts

* garbage collect always-true condition check (commaai#2305)

garbage collect dead code

* CI: use tags for cppcheck update

cppcheck doesn't always create a release for each tag

* lil more

* Adjust `gitversion` handling to include null terminator in length calculations. (commaai#2309)

Co-authored-by: Adeeb Shihadeh <adeebshihadeh@gmail.com>

* fix up fan HITL test (commaai#2317)

* fix up fan HITL test

* cleanup

* times two

* even simpler

* append

* one more...

* speed it up now

* oops

* Build everything before jungle recover (commaai#2316)

* make sure everything is built, including the bootloader

* also for flash

* Align delay and compensate (commaai#2318)

* align delay and compensate

* empty

* rm -rf drivers/spi/; kernel driver isn't needed

* [bot] Update cppcheck to 2.19.1 (commaai#2254)

* [bot] Update cppcheck to 2.19.1

* update coverage table

---------

Co-authored-by: Vehicle Researcher <user@comma.ai>
Co-authored-by: Adeeb Shihadeh <adeebshihadeh@gmail.com>

* Revert "[bot] Update cppcheck to 2.19.1 (commaai#2254)"

This reverts commit ae769db.

* remove mcu_type (commaai#2324)

* remove mcu_type from Panda, it's redundant since all supported devices are H7

* revert disable automatic CAN-FD switching H7 check

* revert original can no longer be flashed error

* assert, assert, assert

* normal reset is fine

* fix usb connect on macos (commaai#2326)

fix claim on macos

* Revert "remove mcu_type (commaai#2324)"

This reverts commit 8922b48.

* remove mcu_type (commaai#2327)

* remove mcu_type from Panda, it's redundant since all supported devices are H7

* revert disable automatic CAN-FD switching H7 check

* revert original can no longer be flashed error

* assert, assert, assert

* normal reset is fine

* remove assert from recover

* update pyproject.toml: include panda.python and panda.board (commaai#2328)

* improve HITL robustness (commaai#2333)

* windows: fix fcntl import (commaai#2329)

* windows: fix fcntl import

* fix indentation

* add windows ci

* make sure CI catches import error

* Revert "make sure CI catches import error"

This reverts commit b18043a.

---------

Co-authored-by: Adeeb Shihadeh <adeebshihadeh@gmail.com>
Co-authored-by: Comma Device <device@comma.ai>
Co-authored-by: Jason Huang <91160588+rexblade21@users.noreply.github.com>
Co-authored-by: Willem Melching <willem.melching@gmail.com>
Co-authored-by: Igor Biletski <briskspirit@users.noreply.github.com>
Co-authored-by: Robbe Derks <robbe.derks@gmail.com>
Co-authored-by: Jason Young <46612682+jyoung8607@users.noreply.github.com>
Co-authored-by: downquark7 <mail2emn@gmail.com>
Co-authored-by: commaci-public <60409688+commaci-public@users.noreply.github.com>
Co-authored-by: Vehicle Researcher <user@comma.ai>
Co-authored-by: Andi Radulescu <andi.radulescu@gmail.com>
mmoo758 added a commit to mmoo758/panda that referenced this pull request Feb 24, 2026
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.

2 participants