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

Revert PR #52 #77

Closed
2 tasks done
dzbaker opened this issue Jul 6, 2023 · 2 comments · Fixed by #78
Closed
2 tasks done

Revert PR #52 #77

dzbaker opened this issue Jul 6, 2023 · 2 comments · Fixed by #78
Milestone

Comments

@dzbaker
Copy link
Contributor

dzbaker commented Jul 6, 2023

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
The changes included in PR #52 introduce bugs. We will revert the PR for this release, then revisit it following release.

Reporter Info
Dylan Z. Baker - NASA/GSFC

dzbaker added a commit that referenced this issue Jul 6, 2023
This reverts commit df7401f, reversing
changes made to da7ef70.
dzbaker added a commit that referenced this issue Jul 6, 2023
Fix #77: Revert merge of PR #52.
@skliper
Copy link
Contributor

skliper commented Jul 6, 2023

@dzbaker - what were the bugs? Documented anywhere?

@dmknutsen
Copy link
Contributor

dmknutsen commented Jul 7, 2023

The change was causing issues with functional testing because the number of characters used (2) in the following calculation is incorrect:
#define MM_MAX_DUMP_INEVENT_BYTES ((CFE_MISSION_EVS_MAX_MESSAGE_LENGTH - (13 + 33)) / 2)

The value should actually be 3 because there is a space in between each byte displayed. The end result is that the message gets truncated and the memory location dumped gets corrupted/lost. Because we are wrapping up the build/past the cut off, and the change is relatively minor, we are trying to punt this to the next build.

Good catch on the documentation - we need to clean up the PR/Issue to make sure we don't accidentally drop this next round.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants