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

Ethernet support #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Ethernet support #1

wants to merge 2 commits into from

Conversation

krame505
Copy link
Member

@krame505 krame505 commented Apr 2, 2018

@JWCS I'm opening a pull request now so I can post comments - this doesn't need to get merged right away.

@@ -77,6 +77,14 @@ This should print the python version and start the python REPL.
```> wine python -c 'import matplotlib; import serial; import typing; import pywintypes'```
The exit code should be 0.

## Repository Layout
* include/Telemetry - Contains Telemetry.h file detailing communication conventions for the Arduino. Must change paramater to switch communication modes between Serial, Ethernet TCP, Ethernet UDP, etc.
Copy link
Member Author

Choose a reason for hiding this comment

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

Typo: paramater->parameter

@@ -77,6 +77,14 @@ This should print the python version and start the python REPL.
```> wine python -c 'import matplotlib; import serial; import typing; import pywintypes'```
The exit code should be 0.

## Repository Layout
Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for adding this.

@@ -9,6 +11,68 @@
#include "Arduino.h"
#include <avr/pgmspace.h>

// Set the protocall here: Serial, Ethernet
#define Protocall Ethernet_TCP
Copy link
Member Author

Choose a reason for hiding this comment

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

Spelling error: protocall -> protocol

Also, this helper stuff generally looks pretty reasonable, although I would request that macros/defines are named as ALL_CAPS, just as a style thing.

@@ -20,28 +84,28 @@
/* { obj.println(); return obj; } */

#define BEGIN_SEND { \
Copy link
Member Author

Choose a reason for hiding this comment

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

For the actual protocol, I've been considering making a significant change to this for Ethernet. The only reason for the weird thing with @@@@@ and &&&&& was a workaround for the issue that we only had one method of output, and I didn't want debugging Serial.print(...) statements to break telemetry.

Since Ethernet doesn't need to include this same sort of debugging flexibility, we could do away with the opening and closing symbols, slightly improving performance. We can change actual status/error messages, currently sent via Serial, to send as just another data field instead.

IDK, just a thought. But this is something we don't need to change right now, I can mess with it later.

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