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.

Sign up for GitHub

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

Jump to bottom

Feature/bsk 64 read c msg #67

Merged
merged 7 commits into from
Jan 6, 2023
Merged

Feature/bsk 64 read c msg #67

merged 7 commits into from
Jan 6, 2023

Conversation

schaubh
Copy link
Contributor

@schaubh schaubh commented Dec 29, 2022 •
edited by patkenneally
Loading

Description

Updated documentation on reading and recording messages. In particular, the issue was raised that a recorder setup on a module C-wrapped input message won't work. The provided change now allows a recorder to be added
to a C-wrapped input module and be thus recorded like the C++ wrapped input messages.

Verification

I did a clean build of the documentation and all looked correct. No RST errors or warnings.

Documentation

Updated the release notes to list the changes in the documentation.

Future work

N/A

@schaubh schaubh linked an issue Dec 29, 2022 that may be closed by this pull request
Subscribed C messages can't be recorded #64
Closed
@schaubh schaubh marked this pull request as draft December 30, 2022 15:56
@schaubh
Copy link
Contributor Author

schaubh commented Jan 2, 2023

added the ability to add a recoder to a C-wrapped input message. All tests passed. Added a unit test for this behavior.

@schaubh schaubh requested a review from patkenneally January 2, 2023 15:54
@schaubh schaubh self-assigned this Jan 4, 2023
schaubh added 3 commits January 5, 2023 09:02
@schaubh
document message read() method
e28dcd8 
Add documentation on how to read the current message values from python.
@schaubh
highlight Recording a Redirected C-wrapped Message
3a796c4 
Highlight in the documentation that recording a C-wrapped message will record the message payload, not where the `payloadPointer` goes.  Thus, if an message
is redirected to write to second message, the recorder needs to be setup on the 2nd message, not the original message.
@schaubh
added release notes
aff337a
@schaubh schaubh force-pushed the feature/bsk_64_read_C_msg branch from 22feae0 to e6796f9 Compare January 5, 2023 16:06
@patkenneally patkenneally self-requested a review January 6, 2023 17:13
patkenneally
patkenneally approved these changes Jan 6, 2023
View reviewed changes
Copy link
Contributor

@patkenneally patkenneally left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Legend:
🔻 Issues to address before merge
🔶 Requests that should not block merge, but should at least be discussed
🔵 Recommendations that can be ignored if desired

Only minor comments. :)

@patkenneally patkenneally marked this pull request as ready for review January 6, 2023 17:14
@schaubh schaubh force-pushed the feature/bsk_64_read_C_msg branch from e6796f9 to 534bcf2 Compare January 6, 2023 18:10
@patkenneally patkenneally self-requested a review January 6, 2023 18:14
@schaubh schaubh merged commit cbd121f into develop Jan 6, 2023
@schaubh schaubh deleted the feature/bsk_64_read_C_msg branch January 6, 2023 19:21
@patkenneally patkenneally added the bug Something isn't working label Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patkenneally patkenneally patkenneally approved these changes

Assignees

@schaubh schaubh

Labels
bug Something isn't working
Projects
Basilisk
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Subscribed C messages can't be recorded
2 participants
@schaubh @patkenneally