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

Fix #123, create package for cfs-groundsystem #137

Merged
merged 1 commit into from
Mar 22, 2021
Merged

Fix #123, create package for cfs-groundsystem #137

merged 1 commit into from
Mar 22, 2021

Conversation

zachar1a
Copy link
Contributor

@zachar1a zachar1a commented Oct 6, 2020

Describe the contribution
A clear and concise description of what the contribution is.

Expected behavior changes
A clear and concise description of how this contribution will change behavior and level of impact.

  • Behavior Change: (The user will be able to type command 'startg' and start the ground system from anywhere on the system)

System(s) tested on

  • OS: [macOS Catalina v 10.15.7]

Additional context
I installed the package with command pip3 install -e relative/path/to/cfs-groundsystem/
In GroundSystem.py there is a variable name ROOTDIR, in order for this command to work, the ROOTDIR has been changed to the path of the file being executed.
The original path was:
ROOTDIR = Path(sys.argv[0]).resolve().parent
and has been changed to:
ROOTDIR = pathlib.Path(file).parent.absolute()

This fix also handles another issue which closes all open windows that spawned from the main process.

Contributor Info - All information REQUIRED for consideration of pull request
Full name and company/organization/center of all contributors ("Personal" if individual work)
Zachary Christian Gonzalez /none/none

  • Else if Individual
    • HAND SIGNED Individual CLA must be on file (once per release): Individual CLA

@lgtm-com
Copy link

lgtm-com bot commented Oct 6, 2020

This pull request introduces 1 alert when merging b386da5 into 10efa70 - view on LGTM.com

new alerts:

  • 1 for Unused import

@skliper skliper added pending CLA enhancement New feature or request labels Oct 7, 2020
@lgtm-com
Copy link

lgtm-com bot commented Oct 8, 2020

This pull request introduces 1 alert when merging d138a0a into 10efa70 - view on LGTM.com

new alerts:

  • 1 for Unused import

@astrogeco
Copy link
Contributor

Hi @zachar1a thank you for your contribution! Could you sign and email the appropriate Contributor License agreement?

email it to: [email protected] and copy [email protected]

Thanks!

@astrogeco
Copy link
Contributor

Got it!

@zachar1a
Copy link
Contributor Author

zachar1a commented Feb 2, 2021

Thank you!

@astrogeco astrogeco changed the title Possible fix for issue #123 by creating package for cfs-groundsystem Fix #123, create package for cfs-groundsystem Feb 3, 2021
@skliper
Copy link
Contributor

skliper commented Mar 5, 2021

@zachar1a please squash and update commit message to match the following form:

Fix #123, Short Description

Long description (optional)

For this case the title of this PR would be perfect. Marking as CCB:Ignore until this is complete (feel free to remove when ready).

@skliper skliper added the CCB:Ignore Pull Request is NOT ready for discussion. Has open actions. Will be re-examined at by next CCB. label Mar 5, 2021
@zachar1a
Copy link
Contributor Author

zachar1a commented Mar 6, 2021

I squashed the commits. However I don't believe I am able to change the labels.

@skliper skliper removed the CCB:Ignore Pull Request is NOT ready for discussion. Has open actions. Will be re-examined at by next CCB. label Mar 8, 2021
@skliper
Copy link
Contributor

skliper commented Mar 8, 2021

@zachar1a Thanks, Looks good. @astrogeco Once reviewed this should be ready for the next round.

@skliper skliper added this to the 3.0.0 milestone Mar 8, 2021
@skliper skliper requested a review from zanzaben March 8, 2021 13:02
@skliper
Copy link
Contributor

skliper commented Mar 8, 2021

@zanzaben - could you also confirm this fixes #121 (and if so link)

@zanzaben
Copy link

zanzaben commented Mar 8, 2021

@zachar1a This isn't working for me on my Ubuntu 20.04.2 LTS machine. The startg command is not found and when I tried running setup.py I got ModuleNotFoundError: No module named 'setuptools'

#121 was fixed and does close all the windows.

@zachar1a
Copy link
Contributor Author

zachar1a commented Mar 8, 2021

A requirements file could be added in to help insure the user has a setuptools install but it will just be that in the file.

And yes for the command to appear you will have to use the pip3 install -e path/to/cFS-GroundSystem

@zanzaben
Copy link

zanzaben commented Mar 9, 2021

It is working for me now.

Can you once again squash your commits and add to your description that you are also fixing issue #121.

@zachar1a
Copy link
Contributor Author

zachar1a commented Mar 9, 2021

My squash didn’t work correctly, it added another commit instead of combining them. Should I try again or just leave it as is?

@zanzaben
Copy link

zanzaben commented Mar 9, 2021

Try again, it needs to be squashed down to a single commit.

@skliper
Copy link
Contributor

skliper commented Mar 9, 2021

Maybe we could share a couple of our common techniques for squashing? There's the interactive rebase, or a soft reset...

@zachar1a
Copy link
Contributor Author

zachar1a commented Mar 9, 2021

I haven't used a soft reset before. But I did try using the interactive rebase this previous time.

@skliper
Copy link
Contributor

skliper commented Mar 9, 2021

Try:

git rebase -i main

and set as

r d672812 Fix #123, create package for cfs-groundsystem Fix #121 close all windows on exit
f af3b10d creating a package for the cfs-groundsystem to be run anywhere on instaalled system with command startg
f dc96b06 Fix #123, create package for cfs-groundsystem
f 9d1b553 fix 123, using a requirements file to insure user has setuptools package needed to install
f 4e64b5e Fix #123, create package for cfs-groundsystem

Then reword the commit to Fix #123, #121, Make into package and close all windows on exit

Or if you want to do the soft reset method, simply:

git reset --soft main
git add -A
git commit -m"Fix #123, #121, Make into package and close all windows on exit"

@skliper skliper linked an issue Mar 9, 2021 that may be closed by this pull request
@zachar1a
Copy link
Contributor Author

zachar1a commented Mar 9, 2021

Thank you very much for the information.

@jphickey
Copy link
Contributor

Just use caution with the "soft" method - you need to make sure you reset back to the original baseline. If you've switched branches and pulled from the remote since starting the branch originally, you might inadvertently overwrite other changes.

Yet another method, although a couple more steps, is pretty straightforward, and no chance of inadvertently overwriting other stuff:

  • Make a new branch with a new name at the current main (OK if main is newer than where you started).
  • Run git merge --squash ${old_branch}
  • Check/Test the result and run git commit if good (this lets you write a new commit message summarizing the full change)
  • Rename your new branch over your old branch (thereby replacing it): git branch -m -f ${new_branch} ${old_branch}
  • Push to github fork as usual

@zachar1a
Copy link
Contributor Author

Thank you for this it will help out a lot in the future.

@astrogeco astrogeco changed the base branch from main to integration-candidate March 22, 2021 13:54
from setuptools import setup

setup(
name='GroundSystem',
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename to "cF-GroundSystem" and update version to use the number in version.py

@astrogeco
Copy link
Contributor

Opening a new issue to add documentation specifically this line:

I installed the package with command pip3 install -e relative/path/to/cfs-groundsystem/

@astrogeco astrogeco merged commit 331b408 into nasa:integration-candidate Mar 22, 2021
astrogeco added a commit to nasa/cFS that referenced this pull request Mar 22, 2021
nasa/cFS-GroundSystem#166 - Updated TBL and SB tlm for an operational TLM display
nasa/cFS-GroundSystem#170 - Add Contributing Guide
nasa/cFS-GroundSystem#137 - Create package for cfs-groundsystem, Fix #
astrogeco added a commit to nasa/cFS that referenced this pull request Mar 22, 2021
nasa/cFE#1243 v6.8.0-rc1+dev436

  nasa/cFE#1225, Add coverage test fix
  nasa/cFE#1218, Adds a local definition of `SOFTWARE_BIG/LITTLE_BIT_ORDER` directly inside `cfe_endian.h` to provide a compatible symbol for apps that still require this. This allows CFE to build and run successfully when OSAL stops providing this in `common_types.h`.
  nasa/cFE#1193, Removes incorrect statements from Application Developers Guide
  nasa/cFE#1235, Fixes truncation handling on vsnprintf error by adding a cast to avoid implicit conversion
  nasa/cFE#1220, Clarify the documentation on SB MsgId regarding requirements for command and telemetry messages
  nasa/cFE#1230, Avoids undefined behavior and resolves static analysis warnings by casting `isspace` input to `unsigned char`.
  nasa/cFE#1231, Updates message module and msgid v1, `CFE_MSG_SetMsgId`, to use mask instead of cast to alter value. Resolves static analysis warning.
  nasa/cFE#1232, Updates `CFE_ES_FileWriteByteCntErr` to report status, not a `size_t` actual since `OS_write` returns `int32`. Use `int16` for local type from `CFE_TBL_FindTableInRegistry` since it's an index, not a status.
  nasa/cFE#1228, Replaces `<>` with `"` in local `#include`s
  nasa/cFE#1227, Adds `CONTRIBUING.md` that links to the main cFS contributing guide.

nasa/PSP#273 v1.5.0-rc1+dev90

  nasa/PSP#264, modular psp implementation
  nasa/PSP#272, Use quotes for local includes
  nasa/PSP#271, Add Contributing Guide

nasa/osal#917 v5.1.0-rc1+dev347

  nasa/osal#890, Move copyblock size to a #define and add comments
  nasa/osal#891, Removed rogue while loop
  nasa/osal#892, Scripted replacement for #include <os and #include <OSC_ matches of < and > with "
  nasa/osal#893, Consolidates the duplicated switch in OS_SocketOpen_Impl
  nasa/osal#894, Add `const` to input pointers
  nasa/osal#895, Removed network prototypes defined in osapi_sockets.h that are also in osapi_network.h
  nasa/osal#896, Removes NULL redefine from common_types.h
  nasa/osal#912, Add Contributing Guide
  nasa/osal#914, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED
  nasa/osal#898, correct interval calculation in DoSelect
  nasa/osal#909, comments describing select after connect
  nasa/osal#902, add check for EAGAIN in addition to EINTR
  nasa/osal#908, compile time assert for sockaddr size
  nasa/osal#910, check/report fcntl status
  nasa/osal#897, Add assert for FD_SET_SIZE in relation to OSAL_set
  nasa/osal#903, better error translation for ESPIPE errno
  nasa/osal#840, add shell functional test
  nasa/osal#901, rename OS_U32ValueWrapper_t
  nasa/osal#900, break up logic in return statement
  nasa/osal#906, return moduleInfoGet error
  nasa/osal#907, report timer_gettime error
  nasa/osal#899, remove unreachable test
  nasa/osal#905, make module comment same as other services
  nasa/osal#920 to fix test error check index inside fdset conversions
  nasa/osal#922, make non-selectable FD an error

nasa/sample_app#137 v1.2.0-rc1+dev54

  nasa/sample_app#134, Convert from <> to " for local includes
  nasa/sample_app#136, Added a contributing guide that links to the main cFS contributing guide.
  nasa/sample_app#132, Add context to the values for MsgIDs

nasa/sample_lib#55 v1.2.0-rc1+dev30

  nasa/sample_lib#54, Replace <> with " for local includes
  nasa/sample_lib#53, Adds CONTRIBUTING.md that links to the main cFS contributing guide.

nasa/cFS-GroundSystem#171 v2.2.0-rc1+dev41

  nasa/cFS-GroundSystem#166, Updated TBL and SB tlm for an operational TLM display
  nasa/cFS-GroundSystem#170, Add Contributing Guide
  nasa/cFS-GroundSystem#137, Create package for cfs-groundsystem
astrogeco added a commit to nasa/cFS that referenced this pull request Mar 22, 2021
nasa/cFE#1243 v6.8.0-rc1+dev436

  nasa/cFE#1225, Add coverage test fix
  nasa/cFE#1218, bit order macros
  nasa/cFE#1193, Removes incorrect statements from Application Developers Guide
  nasa/cFE#1235, Fixes truncation handling on vsnprintf error by adding a cast to avoid implicit conversion
  nasa/cFE#1220, Clarify the documentation on SB MsgId regarding requirements for command and telemetry messages
  nasa/cFE#1230, Cast isspace input to unsigned char to avoid undefined behavior
  nasa/cFE#1231, Updated message module, msgid v1 to use mask instead of cast to alter value
  nasa/cFE#1232, Coercion alters value caused by incorrect type - static analysis warning
  nasa/cFE#1228, Replaces `<>` with `"` in local `#include`s
  nasa/cFE#1227, Adds `CONTRIBUING.md` that links to the main cFS contributing guide.

nasa/PSP#273 v1.5.0-rc1+dev90

  nasa/PSP#264, modular psp implementation
  nasa/PSP#272, Use quotes for local includes
  nasa/PSP#271, Add Contributing Guide

nasa/osal#917 v5.1.0-rc1+dev347

  nasa/osal#890, Move copyblock size to a #define and add comments
  nasa/osal#891, Removed rogue while loop
  nasa/osal#892, Scripted replacement for #include <os and #include <OSC_ matches of < and > with "
  nasa/osal#893, Consolidates the duplicated switch in OS_SocketOpen_Impl
  nasa/osal#894, Add `const` to input pointers
  nasa/osal#895, Removed network prototypes defined in osapi_sockets.h that are also in osapi_network.h
  nasa/osal#896, Removes NULL redefine from common_types.h
  nasa/osal#912, Add Contributing Guide
  nasa/osal#914, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED
  nasa/osal#898, correct interval calculation in DoSelect
  nasa/osal#909, comments describing select after connect
  nasa/osal#902, add check for EAGAIN in addition to EINTR
  nasa/osal#908, compile time assert for sockaddr size
  nasa/osal#910, check/report fcntl status
  nasa/osal#897, Add assert for FD_SET_SIZE in relation to OSAL_set
  nasa/osal#903, better error translation for ESPIPE errno
  nasa/osal#840, add shell functional test
  nasa/osal#901, rename OS_U32ValueWrapper_t
  nasa/osal#900, break up logic in return statement
  nasa/osal#906, return moduleInfoGet error
  nasa/osal#907, report timer_gettime error
  nasa/osal#899, remove unreachable test
  nasa/osal#905, make module comment same as other services
  nasa/osal#920 to fix test error check index inside fdset conversions
  nasa/osal#922, make non-selectable FD an error

nasa/sample_app#137 v1.2.0-rc1+dev54

  nasa/sample_app#134, Convert from <> to " for local includes
  nasa/sample_app#136, Added a contributing guide that links to the main cFS contributing guide.
  nasa/sample_app#132, Add context to the values for MsgIDs

nasa/sample_lib#55 v1.2.0-rc1+dev30

  nasa/sample_lib#54, Replace <> with " for local includes
  nasa/sample_lib#53, Adds CONTRIBUTING.md that links to the main cFS contributing guide.

nasa/cFS-GroundSystem#171 v2.2.0-rc1+dev41

  nasa/cFS-GroundSystem#166, Updated TBL and SB tlm for an operational TLM display
  nasa/cFS-GroundSystem#170, Add Contributing Guide
  nasa/cFS-GroundSystem#137, Create package for cfs-groundsystem
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community enhancement New feature or request
Projects
None yet
5 participants