-
Notifications
You must be signed in to change notification settings - Fork 87
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
Added documentation of Marker Widget and sample UDP Python scripts #240
Conversation
…tation. Included sample Python scripts for UDP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Brian, apologies for taking so long to get this review back to you.
Overall I liked this approach, especially that you found a larger issue with the UDP script in our repo. My only concern is that the documentation you added is potentially repetitive and bloated. Otherwise I think you included all the important info to address the issue.
Thank you again for interviewing with us!
@@ -2,7 +2,7 @@ | |||
id: MotorImagery | |||
title: Motor Imagery | |||
--- | |||
**NOTEe** There are known stumbling blocks with this tutorial. The author is not available for questions. | |||
**NOTE** There are known stumbling blocks with this tutorial. The author is not available for questions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on the typo here
|
||
##### Running the UDP Test | ||
> ⚠ **Important:** | ||
> The **receiver must start before the sender**, otherwise the UDP packets will be lost. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to call this out. However, would look nicer as a tip, for example with the :::info flag you use later on line 452
Received Marker: 1.5585284066901215 | ||
Received Marker: 2.8304827346407384 | ||
<continued...> | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that you include expected outputs for each script
``` | ||
|
||
:::tip | ||
[The Marker Widget supports receiving UDP packets for marking timestamps in data via external hardware](#external-marking-via-udp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't just have to be external hardware. Could also be software that you want to sync with the recording
@@ -317,12 +386,73 @@ In the picture below, you will see an example of some acceptable impedance value | |||
|
|||
## Playback Widget | |||
|
|||
This Widget only appears when in playback mode. It allows you to select a different playback without having to “Stop System”. There is a button in the top right of the Widget that allows you to select any OpenBCI playback file (.txt or .csv). Selecting other types of files may cause an error. | |||
This widget only appears when in playback mode. It allows you to select a different playback without having to “Stop System”. There is a button in the top right of the Widget that allows you to select any OpenBCI playback file (.txt or .csv). Selecting other types of files may cause an error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch on the typo
Open **Terminal 2** and run: | ||
|
||
```sh | ||
python3 -m udp_send_marker --ip 127.0.0.1 --port 12345 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need to include -m flag in these example commands
| `Z` | 5 | | ||
| `X` | 6 | | ||
| `C` | 7 | | ||
| `V` | 8 | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good that you found all the keybinds for these
- IP address input: Defines the target IP for receiving external markers. | ||
- Port input: Specifies the port for receiving UDP marker data. | ||
|
||
 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the included screenshot
This PR addresses #188.
Changes Implemented:
A section on the Marker widget has been added to the Widgets documentations page that explains:
The Networking UDP section has also been updated. Note that this documentation is referencing updated Python sample code within this pull request to the OpenBCI GUI repo: OpenBCI/OpenBCI_GUI#1235
The updates to the Networking UDP section include: