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 aliases #1360

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

fix aliases #1360

wants to merge 5 commits into from

Conversation

Dave9111
Copy link

@Dave9111 Dave9111 commented Feb 5, 2020

This fixes the "no convenience aliases work" networklog, pumploop-log, cat-wifi, etc, issue.
Removed dependency of detection of "sourced" vs "non sourced" script invocation which is not needed and known to be unreliable.
Aliases are written directly into the ~/.bash_profile file so they are easily altered by rig users.
Custom aliases will not be removed or overwritten on subsequent setups.
Repeat setups will not create duplicate aliases.
Tested on a new local install of DEV on an Edison rig.

Rewrite to get rid of dependency on detection of "source" or "non source" invocation of this script, which is not necessary.
Tested on a new install.
Remove arguments from oref0-log-shortcuts function, which is no longer needed after rewrite of that script.
@scottleibrand
Copy link
Contributor

@Dave9111
Copy link
Author

Dave9111 commented Feb 5, 2020

I believe I fixed it..

@scottleibrand
Copy link
Contributor

This is failing unit tests:

# tests/bash-util-functions.test.sh
Actual: This line doesn't get removed

The line below this should be removed
alias networklog="tail -n 100 -F /var/log/openaps/network.log"
The line above this should be removed
Expected: This line doesn't get removed

The line below this should be removed
The line above this should be removed

source "/root/src/oref0/bin/oref0-log-shortcuts.sh"
source /etc/skel/.profile
oref0-log-shortcuts did not modify test_bash_profile correctly

@Dave9111
Copy link
Author

Dave9111 commented Feb 7, 2020

I changed the contents of the .bash_profile to include the aliases.
I didn't understand how the previous oref0-log-shortcuts.sh script could work the way it was written, so I rewrote it so it worked.
It appears that the test is expecting the same thing to be in the .bash_profile as it was before my changes since it is doing a compare to an expected profile. Since I changed the .bash_profile from what it was before, I don't see how the test could possibly pass.
So it appears that the test code is now incorrect since I changed what is now loaded into the .bash_profile?
BTW, I have the same results in the bash-util-function.test.sh on the rig I have here.
How are these tests run?
Is the file bash-util-functions.test.sh the output of the test routine?

@Dave9111
Copy link
Author

Dave9111 commented Feb 8, 2020

Scott,
I reran an installation from my /src directory with the DEV version with the altered oref0-log-shortcuts.sh and the altered oref0-setup.sh file. Then I ran the setup oref0-setup.sh and ran it entirely with the y option at the end of the initial setup.
I edited the ~.bash_profile to clean out the aliases and the "source

~.bash_profile before:


GOROOT=/usr/local/go
export GOROOT
GOPATH=$HOME/go
export GOPATH
PATH=$PATH:/usr/local/go/bin:$GOROOT/bin:$GOPATH/bin
export PATH


After:


GOROOT=/usr/local/go
export GOROOT
GOPATH=$HOME/go
export GOPATH
PATH=$PATH:/usr/local/go/bin:$GOROOT/bin:$GOPATH/bin
export PATH
alias networklog="tail -n 100 -F /var/log/openaps/network.log"
alias xdrip-looplog="tail -n 100 -F /var/log/openaps/xdrip-loop.log"
alias cgm-looplog="tail -n 100 -F /var/log/openaps/cgm-loop.log"
alias autosens-looplog="tail -n 100 -F /var/log/openaps/autosens-loop.log"
alias autotunelog="tail -n 100 -F /var/log/openaps/autotune.log"
alias pump-looplog="tail -n 100 -F /var/log/openaps/pump-loop.log"
alias urchin-looplog="tail -n 100 -F /var/log/openaps/urchin-loop.log"
alias ns-looplog="tail -n 100 -F /var/log/openaps/ns-loop.log"
alias cat-pref="cd /root/myopenaps && cat preferences.json"
alias edit-pref="cd /root/myopenaps && nano preferences.json"
alias cat-wifi="cat /etc/wpa_supplicant/wpa_supplicant.conf"
alias edit-wifi="nano /etc/wpa_supplicant/wpa_supplicant.conf"
alias cat-runagain="cd /root/myopenaps && cat oref0-runagain.sh"
alias edison-battery="cd /root/myopenaps/monitor && cat edison-battery.json"
alias cat-reservoir="cd /root/myopenaps/monitor && cat reservoir.json"
alias stop-cron="cd /root/myopenaps && /etc/init.d/cron stop && killall -g oref$
alias start-cron="/etc/init.d/cron start"
alias tz="sudo dpkg-reconfigure tzdata"
source /etc/skel/.profile
NIGHTSCOUT_HOST=https://xxxxxx.herokuapp.com
export NIGHTSCOUT_HOST
API_SECRET=7e0a14f98cd07e61b3d9babxxxxa76cf63d79809
export API_SECRET
DEXCOM_CGM_RECV_ID=
export DEXCOM_CGM_RECV_ID


Note that this line: "source /etc/skel/.profile"
Was being added before, so I left it as is.
I really don't know why this is in there as there is no ".profile" file in that directory?
It was being added before so I didn't want to mess with it. Let me know if I should do something different with that line. Perhaps a .profile file is created after looping? I don't know. This rig is not able to loop since my daughter is 500 miles away.

I did see a typo in the "edison-battery" alias, so I'll change that and create another PR.

Anyway, I'm not seeing any issues with the before and after. Do you see any problems?

Let me know how I can help resolve the failed tests issues.

Regards, Dave

Fixed typos on "alias edison-battery="   lines
@Dave9111
Copy link
Author

Dave9111 commented Feb 9, 2020

I went back and looked at the test that failed:
Referring to what you sent below:

  1. I broke the test since I got rid of the argument passing - which passed the name of the file to altered. That seemed like overkill to me. Unless I am mistaken, ~/.bash_profile is always in the same place?

  2. The test is broken. From the original oref0-log-shortcuts.sh DEV code these two lines are both in the obsolete alias list and also the new alias list:
    alias networklog="tail -n 100 -F /var/log/openaps/network.log" <--- To be added
    alias networklog="tail -n 100 -F /var/log/openaps/network.log" <-- To be removed
    Yes, they are identical.

Consequently, the existing software would have removed and added it back in, as does my code, and failed the test. I didn't intend to rewrite the list of aliases. But I could. ;-)

So I think the list of aliases should be reviewed, since this alias does not appear to be obsolete.
So if the previous code was working properly, it would have removed the line and added it back in and the test routine would have failed.
My suggestion is to eliminate the test. Either the aliases are added in or not. Its pretty obvious when they are not! Or alter the test to actually check the contents of the ~./bash_profile file? But again, is that not an overkill? Either adding the convenience aliases works or not. I think the test routines should be centered around the critical operations of the software...

Another thing I noticed is that there are more old aliases in the code still. One of the aliases in the new list uses the nano editor, but when invoked the VI editor pulls up the file.

Dave

Test code excerpt:


tests/bash-util-functions.test.sh

Actual: This line doesn't get removed

The line below this should be removed
alias networklog="tail -n 100 -F /var/log/openaps/network.log"
The line above this should be removed
Expected: This line doesn't get removed

The line below this should be removed
The line above this should be removed

source "/root/src/oref0/bin/oref0-log-shortcuts.sh"
source /etc/skel/.profile
oref0-log-shortcuts did not modify test_bash_profile correctly


@scottleibrand
Copy link
Contributor

I’m fine with either fixing or removing the broken test. If you can fix it to actually check if the alises get installed, great. If you can’t figure out a way to do that, removing the useless test would allow us to proceed with fixing the problem it failed to catch.

@scottleibrand scottleibrand changed the title Dev fix aliases Feb 14, 2020
@scottleibrand
Copy link
Contributor

Ok, I just removed the old useless tests that never caught the fact that aliases were broken. This is ready to test on an actual rig.

@scottleibrand
Copy link
Contributor

Updated to this and ran oref0-runagain on a rig without aliases, and it updated .bash_profile, but doesn't seem to cause .bash_profile to be called by .profile.

@scottleibrand
Copy link
Contributor

Not sure if this is related to my non-standard .bashrc or something else. If anyone is setting up a new rig and wants to test it there, that's probably cleaner.

@Dave9111
Copy link
Author

I found this...
https://unix.stackexchange.com/questions/45684/what-is-the-difference-between-profile-and-bash-profile
This could be wrong. But it seems unlikely.
Since Jubilinux is setup to run a /bin/bash shell on login. It appears that on a login, the .bash_profile is executed and if it is not found, .profile is executed? So I'm not sure that .profile is ever executed?
Thanks for finishing this up. Sorry, I've been really busy. I'll load the changes and run it on a newly flashed rig and report back.

@Dave9111
Copy link
Author

I did a install on a newly flashed rig, I ran the standard installation script, then did a git checkout dev, and install with:

cd ~/src/oref0 && git checkout dev && git pull
npm run global-install

Then I ran the setup complete - answering yes.
It did not create a .bash_profile file ?
And the old test code is still in tests/bash-util-functions.test.sh?
And the oref0-log-shortcuts file is the same as weeks ago.
Did I misunderstand something? Did I pull the repo before the commit was really in dev ?
If not, then I am not sure what I did wrong.

I'm really confused why no .bash_profile file was created in setup since the -> touch "$HOME/.bash_profile" <- line is still in the setup script at line 1071?

@scottleibrand
Copy link
Contributor

Sounds like you installed openaps:dev instead of Dave9111:dev. You'll want to do something like:

cd ~/src/oref0 && git checkout dev && git pull
git pull https://github.com/Dave9111/oref0.git dev
npm run global-install

@Dave9111
Copy link
Author

Oops... I misunderstood.
I thought you pushed the changes to openaps dev.
I'll cut out the test code in my rev of dev and try it out.

However, I'm still wondering why .bash_profile wasn't created?

I think I'll try and trap an error by pausing setup just after the touch line.
I can reflash this rig again to verify if I need to verify it works from scratch. Its a spare rig.
Thanks

@Dave9111
Copy link
Author

Dave9111 commented Feb 16, 2020

Scott,

On a newly flashed rig. Running my dev fork, I am getting hung up in the setup script.

I'm getting stuck at about line 862 in oref0-setup.sh. So its not making it to the location (about 1072) where the shortcuts are added.

I don't know why. nightscout is there.. If I run nightscout from the command line it gives me a list of options.

Any idea why this is dying?

I think this is why the .bash_profile file was not being created previously.

I never installed master, only dev. So perhaps there is something missing in dev, that is being setup in master. ??

These are the last few lines in the screen output.


Removing any existing ns device:
removed process://ns/nightscout/ns NIGHTSCOUT_HOST API_SECRET
Running nightscout autoconfigure-device-crud https://leyxxxxxx89.herokuapp.com xxxxxxxxx
added process://ns/nightscout/ns NIGHTSCOUT_HOST API_SECRET
Traceback (most recent call last):
File "/usr/local/bin/openaps-import", line 26, in
class ImportToolApp (cli.ConfigApp):
File "/usr/local/bin/openaps-import", line 37, in ImportToolApp
MAP = get_importable( )
File "/usr/local/bin/openaps-import", line 21, in get_importable
mod = entry.load( )
File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 2188, in load
self.require(env, installer)
File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 2202, in require
items = working_set.resolve(reqs, env, installer)
File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 639, in resolve
raise DistributionNotFound(req)
pkg_resources.DistributionNotFound: mock
Could not run nightscout autoconfigure-device-crud


@Dave9111
Copy link
Author

After I figured out what to do to get an installation with the python issues, this dev version installs cleanly and updates the .bash_profile properly with the aliases. No failed tests, all looks good!

Copy link
Contributor

@scottleibrand scottleibrand left a comment

Choose a reason for hiding this comment

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

oref0-setup exits prematurely after:

Adding OpenAPS log shortcuts
Clearing retrieved apt packages to free space.
Reading package lists... Done
Building dependency tree
Reading state information... Done

Also tried it on another rig and confirmed this is a problem with the branch, not with the rig.

@scottleibrand
Copy link
Contributor

Do you want to try to get this fixed up for 0.7.2-dev, or should we close it out?

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

Successfully merging this pull request may close these issues.

2 participants