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

Implemented optional on screen usage of Surface Dial, reworked some o… #169

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
18 changes: 15 additions & 3 deletions src/HID-APIs/SurfaceDialAPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ typedef union{
struct{
uint16_t button: 1;
uint16_t rotation: 15;
//int8_t xAxis;
//int8_t yAxis;
int8_t xAxis: 8;
int8_t yAxis: 8;

};
} HID_SurfaceDialReport_Data_t;

Expand All @@ -49,17 +49,29 @@ class SurfaceDialAPI
inline void end(void);
inline void click(void);
inline void rotate(int16_t rotation);
inline void position(int8_t x, int8_t y);
Copy link
Contributor

Choose a reason for hiding this comment

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

please review your indentation settings. now it looks like it is wrongly indented.

inline void posrot(int16_t rotation, int8_t x, int8_t y);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would like a more readable name here

inline void press(void);
inline void release(void);
inline void releaseAll(void);
inline bool isPressed();
inline int8_t getX();
inline int8_t getY();
inline bool getOnScreen();
inline void update();

// Sending is public in the base class for advanced users.
virtual void SendReport(void* data, int length) = 0;

protected:
bool _button;
bool _onScreen;
int8_t _xAxis;
int8_t _yAxis;
inline void button(bool b);
inline void xAxis(int8_t x);
inline void yAxis(int8_t y);
inline void onScreen(bool s);
};

// Implementation is inline
Expand Down
77 changes: 68 additions & 9 deletions src/HID-APIs/SurfaceDialAPI.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,39 @@ void SurfaceDialAPI::begin(void)
void SurfaceDialAPI::end(void)
{
_button = false;
rotate(0);
update();
}

void SurfaceDialAPI::click(void)
{
_button = true;
rotate(0);
update();
_button = false;
rotate(0);
update();
}

void SurfaceDialAPI::rotate(int16_t rotation)
{
posrot(rotation, _xAxis, _yAxis);
}

void SurfaceDialAPI::position(int8_t x, int8_t y)
{
posrot(0, x, y);
}

void SurfaceDialAPI::posrot(int16_t rotation, int8_t x, int8_t y)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change this name into something more readable if you intend to keep this function

{
HID_SurfaceDialReport_Data_t report;
_xAxis = x;
_yAxis = y;
report.button = _button;
report.rotation = rotation;
//report.xAxis = x;
//report.yAxis = y;

if(_onScreen)
{
report.xAxis = _xAxis;
report.yAxis = _yAxis;
}
SendReport(&report, sizeof(report));
}

Expand All @@ -64,10 +78,21 @@ void SurfaceDialAPI::button(bool b)
if (b != _button)
{
_button = b;
rotate(0);
update();
}
}

void SurfaceDialAPI::xAxis(int8_t x)
{
_xAxis = x;
posrot(0, _xAxis, _yAxis);
}
void SurfaceDialAPI::yAxis(int8_t y)
{
_yAxis = y;
posrot(0, _xAxis, _yAxis);
}

void SurfaceDialAPI::press(void)
{
button(true);
Expand All @@ -81,10 +106,44 @@ void SurfaceDialAPI::release(void)
void SurfaceDialAPI::releaseAll(void)
{
_button = false;
rotate(0);
update();
}

void SurfaceDialAPI::onScreen(bool s)
{
_onScreen = s;
update();
}

void SurfaceDialAPI::update()
Copy link
Contributor

Choose a reason for hiding this comment

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

Not very intuitive what the difference between posrot and update is. It turns out the difference is in the rotation variable.

{
HID_SurfaceDialReport_Data_t report;
report.button = _button;
report.rotation = 0;
if(_onScreen)
{
report.xAxis = _xAxis;
Copy link
Contributor

Choose a reason for hiding this comment

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

validation of a correct value is not done yet

Copy link
Author

Choose a reason for hiding this comment

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

how should the values be validated?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, I see in the docs that you can specify values out of range to remove the device from the screen

Copy link
Author

Choose a reason for hiding this comment

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

Which Docs say that you can specify values out of the range to remove the device from the screen?

report.yAxis = _yAxis;
}
SendReport(&report, sizeof(report));
}

bool SurfaceDialAPI::isPressed()
{
return _button;
return _button;
}

bool SurfaceDialAPI::getOnScreen()
{
return _onScreen;
}

int8_t SurfaceDialAPI::getX()
{
return _xAxis;
}

int8_t SurfaceDialAPI::getY()
{
return _yAxis;
}
38 changes: 19 additions & 19 deletions src/MultiReport/SurfaceDial.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,25 +50,25 @@ static const uint8_t _hidMultiReportDescriptorSurfaceDial[] PROGMEM = {
0x16, 0xf0, 0xf1, // LOGICAL_MINIMUM (-3600)
0x26, 0x10, 0x0e, // LOGICAL_MAXIMUM (3600)
0x81, 0x06, // INPUT (Data,Var,Rel)
// 0x09, 0x30, // USAGE (X)
// 0x75, 0x10, // REPORT_SIZE (16)
// 0x55, 0x0d, // UNIT_EXPONENT (-3)
// 0x65, 0x13, // UNIT (Inch,EngLinear)
// 0x35, 0x00, // PHYSICAL_MINIMUM (0)
// 0x46, 0xc0, 0x5d, // PHYSICAL_MAXIMUM (24000)
// 0x15, 0x00, // LOGICAL_MINIMUM (0)
// 0x26, 0xff, 0x7f, // LOGICAL_MAXIMUM (32767)
// 0x81, 0x02, // INPUT (Data,Var,Abs)
// 0x09, 0x31, // USAGE (Y)
// 0x46, 0xb0, 0x36, // PHYSICAL_MAXIMUM (14000)
// 0x81, 0x02, // INPUT (Data,Var,Abs)
// 0x05, 0x0d, // USAGE_PAGE (Digitizers)
// 0x09, 0x48, // USAGE (Width)
// 0x36, 0xb8, 0x0b, // PHYSICAL_MINIMUM (3000)
// 0x46, 0xb8, 0x0b, // PHYSICAL_MAXIMUM (3000)
// 0x16, 0xb8, 0x0b, // LOGICAL_MINIMUM (3000)
// 0x26, 0xb8, 0x0b, // LOGICAL_MAXIMUM (3000)
// 0x81, 0x03 // INPUT (Cnst,Var,Abs)
0x09, 0x30, // USAGE (X)
0x75, 0x10, // REPORT_SIZE (16)
0x55, 0x0d, // UNIT_EXPONENT (-3)
0x65, 0x13, // UNIT (Inch,EngLinear)
0x35, 0x00, // PHYSICAL_MINIMUM (0)
0x46, 0xc0, 0x5d, // PHYSICAL_MAXIMUM (24000)
0x15, 0x00, // LOGICAL_MINIMUM (0)
0x26, 0xff, 0x7f, // LOGICAL_MAXIMUM (32767)
0x81, 0x02, // INPUT (Data,Var,Abs)
0x09, 0x31, // USAGE (Y)
0x46, 0xb0, 0x36, // PHYSICAL_MAXIMUM (14000)
0x81, 0x02, // INPUT (Data,Var,Abs)
0x05, 0x0d, // USAGE_PAGE (Digitizers)
0x09, 0x48, // USAGE (Width)
Copy link
Contributor

Choose a reason for hiding this comment

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

Windows expects a value for the width if you want to use the position reporting.

Copy link
Author

Choose a reason for hiding this comment

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

but the width is constant, minimum value is 3000, maximum value is 3000

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

0x36, 0xb8, 0x0b, // PHYSICAL_MINIMUM (3000)
0x46, 0xb8, 0x0b, // PHYSICAL_MAXIMUM (3000)
0x16, 0xb8, 0x0b, // LOGICAL_MINIMUM (3000)
0x26, 0xb8, 0x0b, // LOGICAL_MAXIMUM (3000)
0x81, 0x03 // INPUT (Cnst,Var,Abs)
0xc0, // END_COLLECTION
0xc0 // END_COLLECTION
};
Expand Down