-
Notifications
You must be signed in to change notification settings - Fork 85
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
RawHID support for Teensy based sabers #175
Conversation
…ith the WebUSB app modified: ProffieOS.ino modified: common/stdout.h
modified: ProffieOS.ino
new file: _usb_rawhid.h
#ifdef TEENSYDUINO | ||
#include "common/_usb_rawhid.h" | ||
#if defined(RAWHID_INTERFACE) | ||
class RawHIDParser: Looper, StateMachine { |
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.
Why do we need to duplicate the whole parser?
Isn't it enough to use the existing parser with a different adapter?
@@ -0,0 +1,129 @@ | |||
/* Teensyduino Core Library |
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.
So this looks like it was copied from the teensyduino core library.
Why?
Can't we just use the one that was in there?
If not, we need comments that explains why and what was changed.
@@ -2,7 +2,9 @@ | |||
#define COMMON_STDOUT_H | |||
|
|||
#include "monitoring.h" | |||
|
|||
#ifdef TEENSYDUINO | |||
#include "_usb_rawhid.h" |
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.
Why is the filename starting with an _?
if (stdout_output) { | ||
ret = stdout_output->write(b); | ||
} else { | ||
SerialHID.write(b); |
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.
Why can't we just make SerialHID extend Print?
Alternatively; why can't we just wrap SerialHID in something that extends Print?
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.
That would be the preferred way of implementing it.
I tried to inherit both From Print() and Stream() but did not get it to work (my programming skills failed me to understand why it did not work)
The _usb_rawhid.h inherits from the Teensuiduino usb_rawhid_class RawHID, I tried to extend and managed to get it working the way it is implemented in this PR. The _usb_rawhid.h is my attempt to make the RawHID objetc comply with Stream() to be able to use it in PorffieOS as a serial output, hence my new object _usb_rawusb_class SerialHID
This works;
class rawHIDAdapter {
public:
static void begin() { /* nothing todo / }
// static bool Connected() { return !!WebUSBSerial; }
static bool Connected() { return true; }
static bool AlwaysConnected() { return true; }
static Stream& stream() { return SerialHID; }
static const char response_header() { return "-+=BEGIN_OUTPUT=+-\n"; }
static const char* response_footer() { return "-+=END_OUTPUT=+-\n"; }
};
When I try to use it the way it should;
stdout_output = &SA::stream();
or;
stdout_output = &SerialHID; //reroute output to SerialHID
I get the following error;
...\ProffieOS.ino.cpp.o: In function RawHIDParser::Loop()': ...\ProffieOS/ProffieOS.ino:1572: undefined reference to
SerialHID'
collect2.exe: error: ld returned 1 exit status
I am sure that this could be implemented different to make the SerialHID behave as a Stream* or Print* Although receiving data might prove difficult to get right, and still requires its own parser?
So far I am quite happy that I got it to work, but realise that it is not worthy integrating into the main OS the way it is written right now. I would need some help to get this fixed, as my coding-skills are not enough to see why this is not working the way it should...
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.
There are a few options for how to make this work....
- Start with a copy of usb_seremu.{h,c} from the teensy library, then just modify that to talk to the rawhid endpoint. This would make it work exactly like a serial port, so it would be compatible with everything.
- Don't copy any code, just write a new piece of code in ProffieOS that extends Stream and calls the rawhid recv/send functions to send/receive data. This new class could inherit from Looper to do the polling required. I think this might be slightly more complicated, because there may be some buffere management involved.
Extended the usb_rawhid class provided by Teensyduino an made a SerialHID object /common/_usb_rawhid.h
Could not get it to be an actual Print*, therefore had to add an dedicated serial Parser in ProffieOS.ino and modified common/stdout.h to switch between serialHID and stream objects.
Only when Serial type: raw HID is selected, will the dode be actual used by the compiler.
Any suggestions are welcome to implement it better, before integrating this into ProffieOS