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

Resolves #103 Custom header mechanism #108

Merged

Conversation

lbleier-GSFC
Copy link
Contributor

@lbleier-GSFC lbleier-GSFC commented Jun 29, 2020

Describe the contribution
Fixes #103, implemented the ability to allow users to select the header version byte offset, or custom offsets. In order to support this, some relatively major changes had to be implemented:

Testing performed
Steps taken to test the contribution:

  1. Make changes to code
  2. Run program to verify functionality

Expected behavior changes
The update allows users to change the byte offsets for sending commands and parsing telemetry, to support different header versions or other implementations of cFS

Additional context

  • Note that the "mini" cmdUtil is only a subset of cmdUtil's full functionality. Should the GroundSystem app be updated in the future, the full cmdUtil might need to be used again, or the "mini" cmdUtil updated accordingly
  • There is a larger issue regarding the GroundSystem app as a whole and interprocess communication (IPC). There are multiple techniques to achieve this, but which technique is preferred, and whether or not GroundSystem should be completely redesigned to avoid IPC in the first place, is a discussion for the CCB.

Contributor Info - All information REQUIRED for consideration of pull request
Leor Bleier, NASA GSFC\Code 582

@lbleier-GSFC lbleier-GSFC changed the title Enhance103 headermechanism Enhancement #103 Custom header mechanism Jun 29, 2020
@skliper
Copy link
Contributor

skliper commented Jun 29, 2020

This PR has commits that are already in master. Can it be cleaned up such that we can review the actual change being proposed here?

@lbleier-GSFC lbleier-GSFC force-pushed the enhance103-headermechanism branch 3 times, most recently from 8d3f8be to 1a3b6dc Compare June 29, 2020 13:48
@lgtm-com
Copy link

lgtm-com bot commented Jun 29, 2020

This pull request introduces 2 alerts when merging 1a3b6dc into 63b672a - view on LGTM.com

new alerts:

  • 2 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Jun 29, 2020

This pull request introduces 2 alerts when merging 20fce30 into 63b672a - view on LGTM.com

new alerts:

  • 2 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Jun 29, 2020

This pull request introduces 2 alerts when merging 449511e into 63b672a - view on LGTM.com

new alerts:

  • 2 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Jun 29, 2020

This pull request introduces 2 alerts when merging e37501f into 63b672a - view on LGTM.com

new alerts:

  • 2 for Unused import

@astrogeco astrogeco changed the base branch from integration-candidate to master July 1, 2020 15:37
@astrogeco
Copy link
Contributor

This PR has commits that are already in master. Can it be cleaned up such that we can review the actual change being proposed here?

I think it's addressed now. Which commits did you notice were "duplicates"?

@astrogeco astrogeco added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Jul 1, 2020
@lgtm-com
Copy link

lgtm-com bot commented Jul 1, 2020

This pull request introduces 2 alerts when merging e37501f into defa816 - view on LGTM.com

new alerts:

  • 2 for Unused import

@lbleier-GSFC
Copy link
Contributor Author

I did do some cleanup, but found a few last-minute issues that I pushed. I can squash those further if necessary

@astrogeco
Copy link
Contributor

CCB 2020-07-01: APPROVED

Replace "enhancement" with a valid linking word: Linking a pull request to an issue

Open issue for cross-platform tmp location.

Open issue for ZMQ idea

@lbleier-GSFC lbleier-GSFC changed the title Enhancement #103 Custom header mechanism Resolves #103 Custom header mechanism Jul 1, 2020
@astrogeco astrogeco removed the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Jul 2, 2020
@astrogeco astrogeco changed the base branch from master to integration-candidate July 2, 2020 19:26
@astrogeco
Copy link
Contributor

@lbleier-GSFC if you still have time, can you take a look at the conflicts? If not, maybe @dmknutsen or @JandlynBentley-at-NASA can try

Updates GUI and backend to allow user to select
header version offsets, or custom byte offsets
Updates GUI and backend to allow user to select
header version offsets, or custom byte offsets
Also removed the shareddata.py file; no longer needed
at this time
@lgtm-com
Copy link

lgtm-com bot commented Jul 6, 2020

This pull request introduces 2 alerts when merging fb58248 into aa15da9 - view on LGTM.com

new alerts:

  • 2 for Unused import

@lbleier-GSFC
Copy link
Contributor Author

@lbleier-GSFC if you still have time, can you take a look at the conflicts? If not, maybe @dmknutsen or @JandlynBentley-at-NASA can try

Should be good now

@astrogeco astrogeco removed the conflict label Jul 6, 2020
@astrogeco astrogeco merged commit 658df9f into nasa:integration-candidate Jul 6, 2020
@skliper skliper added this to the 2.2.0 milestone Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants