AVSLab / basilisk

Astrodynamics simulation framework
https://hanspeterschaub.info/basilisk
ISC License
147 stars 61 forks source link

Error on creating message recorders before importing "messaging" #791

Closed juan-g-bonilla closed 2 months ago

juan-g-bonilla commented 2 months ago

Writing this bug report on behalf of @joaogvcarneiro , who originally identified the issue.

Describe the bug Getting any attribute or calling any method of a message object in Python will raise an error if messaging has never been imported from Basilisk.architecture. This is because, when you get a message object in Python from a SWIGed C++ object, SWIG will try to give you the Python version of this message. However, because the messaging module has never imported, these message Python classes have never been declared, so there is no real class to return from C++. Thus, you get an anonymous SwigPyObject that will fail when you get any attribute or method from it.

To reproduce The following script will error out:

from Basilisk.simulation import spacecraft

sc = spacecraft.Spacecraft()
dataLog = sc.scStateOutMsg.recorder()

but not the following:

from Basilisk.simulation import spacecraft
from Basilisk.architecture import messaging

sc = spacecraft.Spacecraft()
dataLog = sc.scStateOutMsg.recorder()

Expected behavior For message objects to always be usable, independent of previously declared imports.

Desktop (please complete the following information):

juan-g-bonilla commented 2 months ago

A possible fix is to import:

from Basilisk.architecture import messaging

in Basilisk/__init__.py, which means that any import from Basilisk would trigger the necessary import.

joaogvcarneiro commented 2 months ago

I'm happy with that fix. Alternatively, we could add it to SimBaseClass - unsure which way is better. Either would do the job.

schaubh commented 2 months ago

I was testing this theory. I noticed that the tutorial script bsk-4.py doesn't import messaging but has no issues using a recorder. Turns out this is because this script imports unitTestSupport.py that import messaging.

Importing messaging in SimulationBaseClass.py does address this issue if this script is loaded. But, in the example above this would not be the case and it still wouldn't work. I know, it would be a very special test script that just loads a BSK module and not simulation base class, but still, the issue remains.

@juan-g-bonilla , where would I put this __init__.py file to include it as you suggest? With your solution every script that imports any BSK related files would have messaging included?

juan-g-bonilla commented 2 months ago

I was testing this theory. I noticed that the tutorial script bsk-4.py doesn't import messaging but has no issues using a recorder. Turns out this is because this script imports unitTestSupport.py that import messaging.

Importing messaging in SimulationBaseClass.py does address this issue if this script is loaded. But, in the example above this would not be the case and it still wouldn't work. I know, it would be a very special test script that just loads a BSK module and not simulation base class, but still, the issue remains.

@juan-g-bonilla , where would I put this __init__.py file to include it as you suggest? With your solution every script that imports any BSK related files would have messaging included?

@schaubh You can edit it your existing installation by going to dist3/Basilisk/__init__.py and appending to that file (whatever its contents, depends on the platform) the line:

from Basilisk.architecture import messaging

That __init__.py file is actually autogenerated by the build system, so we'd have to update the CMakeLists.txt: https://github.com/AVSLab/basilisk/blob/2588500528755979bec855ed721af04959142676/src/CMakeLists.txt#L648-L654

schaubh commented 2 months ago

Just tried this and the solution works well. I'll add release notes to my branch and put up a PR.