Skip to content

Conversation

@Lytrix
Copy link
Collaborator

@Lytrix Lytrix commented Dec 31, 2017

Added option to the command line to write what you have done into a json file after closing the omni_rfcat_read with ctrl+c. The file contains all the raw and string messages you've been receiving, to use it more easily with copy pasting in the wiki.

Copy link
Collaborator Author

@Lytrix Lytrix left a comment

Choose a reason for hiding this comment

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

.

# Default variables for filename
dateRecording = datetime.today().isoformat()
userName = getpass.getuser()
fileName = '%s-%s' % (dateRecording, userName)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am also looking for a better way of naming the files. Now there only date and username, so finding your results back is a bit cumbersome. The free text input would be a bit to unorganised as a filename I think. Do you have any suggestions?

"""Create 1 dict for each message loop."""
package ={}
messages = []
messages = {"string": [],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've grouped the raw's all together and the strings, I first had it grouped for each step, but that was not really easy to read, see added example with the grouping I find is really helpful. @ps2 what do you think?

git clone https://github.com/openaps/openomni.git
cd openomni
pip install -e .
pip install -e . --process-dependency-links
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this necessary for rfcat? Reading about the security implications and how they want to deprecate this feature, I think maybe it should be avoided. We can list installation of rfcat as a separate step.

Copy link
Collaborator Author

@Lytrix Lytrix Jan 13, 2018

Choose a reason for hiding this comment

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

For RFcat this is necessary if you want to install it with setup.py because it not registered on PiPy.

I read the option has been put back in because there is currently no other solution in the works:
pypa/pip#4187

I would like to keep it in for now because it simplifies the install steps, but I will create PR tonight on RFCat with the needed changes to add rfcat to the PiPy registry. It's quite easy to add packages to pipy yourself.

The separate step was a bit confusing for me at the time to understand what RFCat is used for. I used the RFCat first as a CC compiler for the TI stick and then also for openomni as a python package. So when I found out it can be installed as a package, I liked this, because it helps in the logic of all the different steps and have a 1 install command (I started using Docker at work so I am really into 1 line installers now ;-))

sys.stdin.read(1)

# Default variables for filename
dateRecording = datetime.today().isoformat()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use python recommended naming for variables: underscores instead of camel case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done!



#actionDescription = raw_input("Write what you are doing: ")
with open('../../results/' + fileName + '.json', 'a') as outFile:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This fails for me. I usually work out of the top level directory, and run things like ./openomni/bin/omni_listen_rfcat. File output should be an option; one is not always wanting to store things to disk.

Copy link
Collaborator Author

@Lytrix Lytrix Jan 13, 2018

Choose a reason for hiding this comment

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

Good call. I've added a path argument to tackle this, plus added it to the readme. I will add the option to skip writing to file.

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