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

Add serial_test.py #28

Merged
merged 15 commits into from
Jan 5, 2020
Merged

Add serial_test.py #28

merged 15 commits into from
Jan 5, 2020

Conversation

hallgchris
Copy link
Member

@hallgchris hallgchris commented Oct 7, 2019

Basically mqtt_test.py but imitates the serial data coming from the teensy. To test:

python serial_test.py -f data_173.csv -j 1500 (or whatever file you want)

Then in another console,

node DAS.js -a -p [slave port] (slave port is printed by the previous statement)

Then press enter in the first console window. You can confirm it's working if you see data on mosquitto_sub -t data or on the DAShboard.

Tested on linux, should work on macOS, don't think it will work on windows.

@khanguslee
Copy link
Contributor

Will need to test this out. But on a quick glance, this is looking good. Only gripe I have is to rename test_data_generator.py to something else as it may run as a test script from pytest or similar due to the test_*.py filename convention.

harsilspatel
harsilspatel previously approved these changes Oct 8, 2019
Copy link
Contributor

@harsilspatel harsilspatel left a comment

Choose a reason for hiding this comment

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

looks great, nice work bikie! @hallgchris ^_^

Copy link
Contributor

@khanguslee khanguslee left a comment

Choose a reason for hiding this comment

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

Have an issue with running the script. Might be good to update the README.md on how to run this script.

Raspi/Test/serial_test.py Show resolved Hide resolved
Raspi/Test/serial_test.py Show resolved Hide resolved
@khanguslee
Copy link
Contributor

If this is working on linux, happy to just accept this but with an OS check that only allows the test script to work on linux only. We'll worry about getting this to work for other OS' in another task :)

Thoughts?

@hallgchris
Copy link
Member Author

If this is working on linux, happy to just accept this but with an OS check that only allows the test script to work on linux only. We'll worry about getting this to work for other OS' in another task :)

Thoughts?

Happy with that for now. I'm not really sure if I'll be able to work out a fix without having a mac to experiment on, so otherwise the PR will be sitting here indefinitely

@hallgchris
Copy link
Member Author

Pretty sure everything's fine but gotta get reviewed again to merge 😅

emilytrau
emilytrau previously approved these changes Jan 4, 2020
Copy link

@emilytrau emilytrau left a comment

Choose a reason for hiding this comment

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

Looks good to me

harsilspatel
harsilspatel previously approved these changes Jan 5, 2020
Copy link
Contributor

@harsilspatel harsilspatel left a comment

Choose a reason for hiding this comment

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

as always, great work chris! :D

parser.add_argument('-r', '--rate', action='store', type=float, default=0.5, help="rate of data in seconds")
parser.add_argument('--host', action='store', type=str, default="localhost", help="address of the MQTT broker")
parser.add_argument('-f', '--file', action='store', type=str, help="The csv file to replay. If not specified, makes up data.")
parser.add_argument('-j', '--jump', action='store', type=int, default=0, help="Starts replaying from a specified time (in seconds)")
Copy link
Contributor

Choose a reason for hiding this comment

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

i just love how the arguments also have shorthands!

@hallgchris
Copy link
Member Author

Sorry guys need another review 😂

Copy link

@emilytrau emilytrau left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@hallgchris hallgchris merged commit 9ed61a3 into master Jan 5, 2020
@hallgchris hallgchris deleted the hallgchris/serial_test branch January 5, 2020 09:56
@hallgchris
Copy link
Member Author

Finally lol

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.

4 participants