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.

Sign up for GitHub

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

Jump to bottom

Force effect corrections part1 #105

Merged
merged 3 commits into from
Jun 20, 2024
Merged

Force effect corrections part1 #105

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c2bc4fe
correct constant force level
MmAaXx500 Jun 16, 2024
a6b104c
fix periodic effect magnitude and phase calculation
MmAaXx500 Jun 19, 2024
5d3ca03
fix periodic effect offset calculation
MmAaXx500 Jun 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 48 additions & 14 deletions src/tmt300rs/hid-tmt300rs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,32 @@ static u8 damper_values[] = {
0x7f, 0x07
};

static void t300rs_calculate_periodic_values(struct ff_effect *effect)
{
struct ff_periodic_effect *periodic = &effect->u.periodic;
int16_t headroom;

effect->replay.length -= 1;

periodic->magnitude = (periodic->magnitude * fixp_sin16(effect->direction * 360 / 0x10000)) / 0x7fff;

if (periodic->magnitude < 0){
/* the wheel handles positive magnitudes only */
periodic->magnitude = -periodic->magnitude;

/* to give the expected result 180 deg is added to the phase */
periodic->phase = (periodic->phase + (0x10000 / 2)) % 0x10000;
}

/* the interval [0; 32677[ is used by the wheel for the [0; 360[ degree phase shift */
periodic->phase = periodic->phase * 32677 / 0x10000;
Copy link
Owner

Choose a reason for hiding this comment

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

32677 is a bit surprising, I would've expected it to be 32768. I guess it could be that the driver is trying to avoid some unwanted behaviour with values at the extreme ranges?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it maps to some lookup table inside the wheel, although the value choice still odd, but it works out perfectly.
Wine has some issues using the correct values through the whole range, especially from 390 to 399 deg. I don't know why it happens, but it had it before too (see spreadsheet).

Copy link
Owner

Choose a reason for hiding this comment

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

32677 + 91 = 32768, I guess the Windows driver just doesn't use the last interval or something.

but it works out perfectly.

Does 32768 still work, or did you mean that the 32677 maximum is the highest working value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we assume the Windows driver is working correctly, then [0; 32677[ -> [0; 360[ and [0; 65354[ -> [0; 720[.
The wheel responds to the whole [0, 65535] interval.


headroom = 0x7FFF - periodic->magnitude;
/* magnitude + offset cannot be outside the valid magnitude range, */
/* otherwise the wheel behaves incorrectly */
periodic->offset = clamp(periodic->offset, -headroom, headroom);
}

int t300rs_send_buf(struct t300rs_device_entry *t300rs, u8 *send_buffer, size_t len)
{
int i;
Expand Down Expand Up @@ -630,6 +656,8 @@ static int t300rs_update_constant(struct t300rs_device_entry *t300rs,
int16_t level;

level = (constant.level * fixp_sin16(effect.direction * 360 / 0x10000)) / 0x7fff;
/* the Windows driver uses the range [-16385;16381] */
level = level / 2;

if ((constant.level != constant_old.level) || (effect.direction != old.direction)) {

Expand Down Expand Up @@ -826,26 +854,30 @@ static int t300rs_update_spring(struct t300rs_device_entry *t300rs,
return t300rs_update_damper(t300rs, state);
}


static int t300rs_update_periodic(struct t300rs_device_entry *t300rs,
struct tmff2_effect_state *state)
{
struct ff_effect effect = state->effect;
struct ff_effect old = state->old;
struct ff_periodic_effect periodic = effect.u.periodic;
struct ff_periodic_effect periodic_old = old.u.periodic;
struct __packed t300rs_packet_mod_periodic {
struct t300rs_packet_header header;
uint8_t attribute;
uint16_t value;
} *packet_mod_periodic = (struct t300rs_packet_mod_periodic *)t300rs->send_buffer;

struct ff_periodic_effect periodic, periodic_old;
int ret;
int16_t magnitude, phase;
uint16_t phase;
int16_t magnitude;

magnitude = (periodic.magnitude * fixp_sin16(effect.direction * 360 / 0x10000)) / 0x7fff;
t300rs_calculate_periodic_values(&effect);
periodic = effect.u.periodic;
magnitude = periodic.magnitude;
phase = periodic.phase;

t300rs_calculate_periodic_values(&old);
periodic_old = old.u.periodic;

if ((periodic.magnitude != periodic_old.magnitude)
|| (effect.direction != old.direction)) {

Expand Down Expand Up @@ -948,6 +980,8 @@ static int t300rs_upload_constant(struct t300rs_device_entry *t300rs,
int ret;

level = (constant.level * fixp_sin16(effect.direction * 360 / 0x10000)) / 0x7fff;
/* the Windows driver uses the range [-16385;16381] */
Copy link
Owner

Choose a reason for hiding this comment

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

The previous comment also applies to this, even more weird is the range being asymmetric.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an about -2 offset on the whole range in the constant force magnitude and in the periodic effect offset.
The Windows driver using -2 instead of 0 force/offset in these cases and never uses 0 (see spreadsheet).

I have not replicated this because I think it's a rounding error and saw no difference in behavior.

level = level / 2;
duration = effect.replay.length - 1;

offset = effect.replay.delay;
Expand Down Expand Up @@ -1121,7 +1155,6 @@ static int t300rs_upload_periodic(struct t300rs_device_entry *t300rs,
struct tmff2_effect_state *state)
{
struct ff_effect effect = state->effect;
struct ff_periodic_effect periodic = state->effect.u.periodic;
struct __packed t300rs_packet_periodic {
struct t300rs_packet_header header;
uint16_t magnitude;
Expand All @@ -1134,19 +1167,20 @@ static int t300rs_upload_periodic(struct t300rs_device_entry *t300rs,
struct t300rs_packet_timing timing;
} *packet_periodic = (struct t300rs_packet_periodic *)t300rs->send_buffer;

struct ff_periodic_effect periodic;
int ret;
uint16_t duration, period, offset, periodic_offset;
int16_t phase, magnitude;

duration = effect.replay.length - 1;
magnitude = (periodic.magnitude * fixp_sin16(effect.direction * 360 / 0x10000)) / 0x7fff;
uint16_t duration, period, phase, offset, periodic_offset;
int16_t magnitude;

t300rs_calculate_periodic_values(&effect);
periodic = effect.u.periodic;
duration = effect.replay.length;
offset = effect.replay.delay;
magnitude = periodic.magnitude;
period = periodic.period;
phase = periodic.phase;
periodic_offset = periodic.offset;

period = periodic.period;
offset = effect.replay.delay;

t300rs_fill_header(&packet_periodic->header, effect.id, 0x6b);

packet_periodic->magnitude = cpu_to_le16(magnitude);
Expand Down