Skip to content

flatsat & canbus: Create flatsat app with CANBus implementation#147

Open
TheArchons wants to merge 3 commits intomainfrom
can-bus
Open

flatsat & canbus: Create flatsat app with CANBus implementation#147
TheArchons wants to merge 3 commits intomainfrom
can-bus

Conversation

@TheArchons
Copy link
Member

@TheArchons TheArchons commented Dec 20, 2025

main.c contains code of a working CANBus implmenentation that sends/recieves messages to/from the RaspberryPi on the flatsat.

The main issue was the device tree, which I needed to update to support CAN. The external clock (hse) didn't work so I needed to use the internal one (hsi). I also changed pll to use hsi instead of hse. hse works, the issue was that you needed to manually specify which clocks to use.

@TheArchons TheArchons added the DNM DO NOT MERGE (ONLY THOSE WHO PUT THIS LABEL CAN REMOVE IT) label Dec 20, 2025
@TheArchons TheArchons force-pushed the can-bus branch 11 times, most recently from 98229de to d13a7a4 Compare December 20, 2025 21:04
Copy link
Member

@mshinjo mshinjo 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 general comment and I haven’t fully reviewed the changes yet, but:

  1. Can we create separate patches for different areas? For example, hardware definitions (devicetree) should be in their own patch.
  2. Can we update the CI workflow to build the new app as well if we are adding a new app
  3. Is apps/can-bus/prj.conf intentional? What is its purpose? Is it meant to be used with -DCONF?

@TheArchons TheArchons force-pushed the can-bus branch 2 times, most recently from 8d54a19 to 6f5c569 Compare December 20, 2025 21:13
@TheArchons
Copy link
Member Author

Can we create separate patches for different areas? For example, hardware definitions (devicetree) should be in their own patch.

Just a general comment and I haven’t fully reviewed the changes yet, but:

1. Can we create separate patches for different areas? For example, hardware definitions (devicetree) should be in their own patch.

You're right, I'll split this into two PRs

2. Can we update the CI workflow to build the new app as well if we are adding a new app

Sure

3. Is apps/can-bus/prj.conf intentional? What is its purpose? Is it meant to be used with -DCONF?

Unintentional, fixed

@TheArchons TheArchons force-pushed the can-bus branch 3 times, most recently from 6fc4665 to 9e69dad Compare December 20, 2025 21:26
@mshinjo
Copy link
Member

mshinjo commented Dec 20, 2025

You're right, I'll split this into two PRs

It doesn't have to be a separate PR, but a separate patch (commit)
This should PR should contain three patches: one to modify dts, one to add a new app, and one to modify CI

@mshinjo
Copy link
Member

mshinjo commented Dec 20, 2025

The main issue was the device tree, which I needed to update to support CAN. The external clock (hse) didn't work so I needed to use the internal one (hsi). I also changed pll to use hsi instead of hse.

Do you know if it booted at all?

@TheArchons
Copy link
Member Author

The main issue was the device tree, which I needed to update to support CAN. The external clock (hse) didn't work so I needed to use the internal one (hsi). I also changed pll to use hsi instead of hse.

Do you know if it booted at all?

What do you mean by it? can_start didn't have any issues with hse, but the timings must have been off or something since it was never received by the RPI. Might be worth looking into why, but I couldn't figure it out.

@chopikus
Copy link
Contributor

So @TheArchons are you using HSE or HSI? The last commit suggests you are still using HSI. Also the copilot wants to change it to HSE? Can you update the commit message that HSE works? Feel free to add more info why did you think it didn't work (maybe it was a different configuration). This would help a lot to someone who will work on it later

@chopikus
Copy link
Contributor

Otherwise I'm glad it works 🔥

@x4132
Copy link

x4132 commented Jan 18, 2026

lgtm

also imagine force pushing, skill issue

@TheArchons
Copy link
Member Author

So @TheArchons are you using HSE or HSI? The last commit suggests you are still using HSI. Also the copilot wants to change it to HSE? Can you update the commit message that HSE works? Feel free to add more info why did you think it didn't work (maybe it was a different configuration). This would help a lot to someone who will work on it later

Oh yea I forgot to change the commit messages, thanks.

This provides the configuration needed for CANBus to work on obc.

Signed-off-by: Alexander Li <github@thearchons.xyz>
alexapostolu
alexapostolu previously approved these changes Feb 8, 2026
@alexapostolu alexapostolu removed the DNM DO NOT MERGE (ONLY THOSE WHO PUT THIS LABEL CAN REMOVE IT) label Feb 8, 2026
TheArchons added a commit that referenced this pull request Feb 8, 2026
This is mostly the same as the OBC dts in #147, but for PAY.

Signed-off-by: Alexander Li <github@thearchons.xyz>
Copy link
Member

@mshinjo mshinjo left a comment

Choose a reason for hiding this comment

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

Have we decided between using a separate flatsat image versus developing in the flight software app?

alexapostolu pushed a commit that referenced this pull request Feb 8, 2026
This is mostly the same as the OBC dts in #147, but for PAY.

Signed-off-by: Alexander Li <github@thearchons.xyz>
@TheArchons TheArchons force-pushed the can-bus branch 5 times, most recently from 40c2a2a to 2bc1fe7 Compare March 12, 2026 00:25
I am not sure why this build didn't fail before, but we need to define
the CAN configuration for the g431rb.

Signed-off-by: Alexander Li <github@thearchons.xyz>
Comment on lines +11 to +74
LOG_MODULE_REGISTER(flatsat);

const struct device *const can = DEVICE_DT_GET(DT_CHOSEN(zephyr_canbus));

void pi_rx_callback(const struct device *dev, struct can_frame *frame, void *user_data)
{
LOG_INF("RECEIVED CAN MESSAGE WITH ID [%x]", frame->id);

const int dlc = frame->dlc;

for (int i = 0; i < dlc; i++) {
LOG_INF("%x", frame->data[i]);
}
}

int main(void)
{
while (1) {
LOG_INF("obc");
k_msleep(1000);
int ret;

LOG_INF("Starting CAN send");

LOG_INF("DEVICE IS READY: %d", device_is_ready(can));

if (IS_ENABLED(CONFIG_CAN_MODE_LOOPBACK)) {
// use loopback mode (send to itself) for debugging
ret = can_set_mode(can, CAN_MODE_LOOPBACK);
if (ret != 0) {
LOG_ERR("Error setting loopback mode [%d]", ret);
return 0;
}
LOG_INF("Loopback mode enabled");
}

ret = can_start(can);

LOG_INF("CAN STARTED: %d", ret);

// what to filter for regarding receiving messages
const struct can_filter pi_filter = {
.flags = 0U,
.id = 0x100, // when receiving messages from the pi this must match (e.g. to send 100#ABCD .id needs to be 0x100)
.mask = CAN_STD_ID_MASK
};

can_add_rx_filter(can, &pi_rx_callback, NULL, &pi_filter);

while (true) {
struct can_frame frame = {
.flags = 0,
.id = 0x100,
.dlc = 4, // data length. Must match the number of elements in .data
.data = { 0xde, 0xad, 0xbe, 0xef }
};
LOG_INF("Waiting 5 seconds");
k_msleep(5000);
ret = can_send(can, &frame, K_MSEC(3000), NULL, NULL);
if (ret) {
LOG_ERR("Sending failed [%d]", ret);
continue;
}

LOG_INF("CAN sent successfully");
}

Copy link
Member

Choose a reason for hiding this comment

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

This isn't OBC specific, so I would say that we clean this up and put it under lib/, and create a thread in apps.

In apps, only thread creations and app specific init code should live.
So in apps, in terms of number of lines, it should be short. My guess is that it would be max 200 lines.

Also, we really should start using libcsp. The CAN transportation shouldn't be happening at the Zephyr API layer. Otherwise, we will starting to create technical debt... and it would be hard to fix and clean up... (which happened before the Zephyr introduction)

@alexapostolu FYI I have some libcsp stuff, which should be PR ready this weekend.

Copy link
Member

Choose a reason for hiding this comment

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

@mshinjo Are you going to be in the lab on Saturday? We can talk about this more and get the libcsp stuff merged too

Copy link
Member

Choose a reason for hiding this comment

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

@alexapostolu 50/50 not sure yet. I can probably visit around 5PM. Talking in-person would be way easier and faster.

main.c contains a working CANBus implementation that sends and
receives messages to/from the Raspberry Pi on the flatsat.

Signed-off-by: Alexander Li <github@thearchons.xyz>
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.

8 participants