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

Feature/466 running non swig v2 #652

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

spiggottCO
Copy link
Contributor

@spiggottCO spiggottCO commented Mar 20, 2024

  • Tickets addressed: bsk-466
  • Review: By commit
  • Merge strategy: Merge (no squash)

Description

This branch makes it possible to use "foreign" modules inside Basilisk. In this case, foreign means that the module does not have to confirm to the Basilisk framework, particularly that SWIG is not required. It uses address access to be able to attach modules and messages to the Basilisk simulation context.

Verification

The verification for this update is basically that the modules do not break the existing use cases. However, going forward, non-SWIG modules (using pybind for example) could be added to Basilisk along with the associated dependencies.

Documentation

Undocumented. This is the deep dark heart of Basilisk.

Future work

I'm doing most of my research using pybind, and we can talk about adding those modules back into Basilisk using this mechanism if it makes sense.

@spiggottCO spiggottCO linked an issue Mar 20, 2024 that may be closed by this pull request
@schaubh schaubh self-requested a review March 23, 2024 13:20
@schaubh schaubh self-assigned this Mar 23, 2024
@schaubh schaubh added the enhancement New feature or request label Mar 23, 2024
Copy link
Contributor

@schaubh schaubh left a comment

Choose a reason for hiding this comment

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

  • edit commit comment on firsts commit
  • add requested code comments and method descriptions
  • add a unit test that checks if this work if we use this to connect with regular BSK messages. At least we are then checking the basic operation of this functionality. Add unit test to src/architecture/messaging/_UnitTest
  • add release notes briefly describing what new functionality is added to docs/source/Support/ReleaseNotes.rst
  • rebase on latest develop

@@ -14,6 +14,8 @@ typedef struct {type};
{type}_C_subscribe(self, source)
elif type(source) == {type}:
{type}_cpp_subscribe(self, source)
elif type(source) == int:
Copy link
Contributor

Choose a reason for hiding this comment

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

add a comment why you are checking if this is an int

@@ -26,6 +28,8 @@ typedef struct {type};
return ({type}_C_isSubscribedTo(self, source))
elif type(source) == {type}:
return ({type}_cpp_isSubscribedTo(self, source))
elif type(source) == int:
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, add comment on why you are checking for int.

@@ -107,6 +107,10 @@ void {type}_cpp_subscribe({type}_C *subscriber, void* source){{
subscriber->headerPointer->isLinked = 1; // set output message as linked
}};

void {type}_addr_subscribe({type}_C *subscriber, uint64_t sourceAddr){{
Copy link
Contributor

Choose a reason for hiding this comment

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

add a description of this method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

Running non-SWIG modules in a Basilisk simulation/context
2 participants