Skip to content
This repository was archived by the owner on Apr 17, 2025. It is now read-only.

Conversation

sphw
Copy link
Contributor

@sphw sphw commented May 8, 2024

This PR adds a simple wrapper around basilisk flight software modules. It replaces the basilisk message passing interface with one based on flume. The goal is to allow us to build wrappers around basilisk FSW algorithms, leaving them unchanged.

Copy link

linear bot commented May 8, 2024

@sphw sphw requested review from akhilles and ch-greams May 8, 2024 00:12
@sphw sphw force-pushed the feature/basilisk-rs branch from d9ddfe4 to d029beb Compare May 8, 2024 02:52
@sphw sphw force-pushed the feature/basilisk-rs branch from b454e77 to 659d8f5 Compare May 8, 2024 17:46
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be deleted?

Copy link
Contributor

Choose a reason for hiding this comment

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

Empty file

vehicle_config,
);
mrp_feedback.reset(0);
mrp_feedback.update(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

I expect that a single FSW process would compose a set of basilisk modules and then run update() on them sequentially. Is that accurate? If so, could we use a simpler message passing solution like a mailbox?

I'm a bit skeptical of using flume in such a critical path. It has had a bunch of nasty bugs in the past, and the codebase is hard to audit because it's so expansive in scope. For running modules in parallel, we'd still use iceoryx.

Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, this isn't be blocker. I just think we should reconsider this as part of the hardening process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH I just reached for it out of habit more than anything. I think this is actually a perfect place to use https://docs.rs/thingbuf/latest/thingbuf/mpsc/index.html

Copy link
Contributor

Choose a reason for hiding this comment

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

https://docs.rs/thingbuf/latest/thingbuf/struct.StaticThingBuf.html seems ideal if we can get away with not blocking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah StaticThingBuf makes sense to me, we could keep it bounded with a very small capacity. Basilisk expects us not to block, so we can just use try_recv

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also has the nice added side-effect of making this code no_std compatible right of the bat

Copy link
Contributor Author

@sphw sphw May 8, 2024

Choose a reason for hiding this comment

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

Replaced flume with the non-static thingbuf in this PR. The static thingbuf has a slightly annoying property in that it basically needs to be defined in static var. That might be ok, but. I'm not sure so I'm going with the more flexible option for now

let channel: *mut BskChannel<$payload_name> =
unsafe { std::mem::transmute(channel) };
let channel: &mut BskChannel<$payload_name> = unsafe { &mut *channel };
channel.read().unwrap_or_default()
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious what basilisk expects from the read/write calls. Is it ok to block or fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The modules expect that you always return a value without blocking, even if it's stale. That makes the most since for their model since the modules are run in a single threaded loop, and so you wouldn't want them to block each other. The current impl mimics that behavior using that last_msg we store

@sphw sphw merged commit 852bd0f into main May 8, 2024
@sphw sphw deleted the feature/basilisk-rs branch May 8, 2024 22:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants