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

Possible Error in code of orientation #36

Open
z0mb0lix opened this issue Apr 16, 2015 · 2 comments
Open

Possible Error in code of orientation #36

z0mb0lix opened this issue Apr 16, 2015 · 2 comments

Comments

@z0mb0lix
Copy link

Hey pfps.

I am almost certain I found a small error in your orientation.c code that prevents the auto rotation:

406 if (previous_orientation == orientation /* only rotate when stable */ && orientation !=
407 screen_orientation && orientation != FLAT && !orientation_lock) {
408 rotate_to(orientation);
409 previous_orientation = orientation;
410
411 }

In line 406 the dependencies to execute the rotate_to - section are defined as

if (previous_orientation == orientation /* only rotate when stable */ &&
orientation != screen_orientation && orientation != FLAT && !orientation_lock)

If I understand it correct, the value of previous_orientation is defined as "-1" in the header section and gets a new value at two positions:

  1. after the dependencies above are met and rotate_to sequence is executed (Line 409) it gets the same value as orientation,
  2. during the sigusr_callback_handler sequence (Line 225) it gets the same value as screen_orientation.

BUT, here comes the clue: screen_orientation also only gets his value in the rotate_to sequence.
But that prevents the section from actually running because the algorithms to set the values to fulfil the dependencies are only executed AFTER the dependencies are fulfilled.
It SHOULD (imo) set the value of previous_orientation after each time the check orientation process is finished.

So.. long story short: I think the previous_orientation=orientation command should be taken out of the sequence above and placed as a dependencie-free command behind the rest

Something like this:
406 if (previous_orientation == orientation /* only rotate when stable */ && orientation !=
407 screen_orientation && orientation != FLAT && !orientation_lock) {
408 rotate_to(orientation);
409
410 }
411 previous_orientation = orientation;

I tried it with this code and if "run sequence once" - write previous_orientation at the end after all other commands - "run sequence again" - previous_orientation=orientation at the beginning - Hurray!

Greetings

Felix

@pfps
Copy link
Owner

pfps commented Apr 16, 2015

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I'll take a look shortly.

peter

On 04/16/2015 05:03 AM, z0mb0lix wrote:

Hey pfps.

I am almost certain I found a small error in your orientation.c code
that prevents the auto rotation:

406 if (previous_orientation == orientation /* only rotate when stable */
&& 407 orientation != screen_orientation && orientation != FLAT && 408
!orientation_lock) { 409 rotate_to(orientation); 410 previous_orientation
= orientation; 411 }

In line 406 the dependencies to execute the rotate_to - section are
defined as

if (previous_orientation == orientation /* only rotate when stable */ &&
orientation != screen_orientation && orientation != FLAT &&
!orientation_lock)

If I understand it correct, the value of previous_orientation is defined
as "-1" in the header section and gets a new value at two positions: 1)
after the dependencies above are met and rotate_to sequence is executed
(Line 409) it gets the same value as orientation, 2) during the
sigusr_callback_handler sequence (Line 225) it gets the same value as
screen_orientation.

BUT, here comes the clue: screen_orientation also only gets his value in
the rotate_to sequence. But that prevents the section from actually
running because the algorithms to set the values to fulfil the
dependencies are only executed AFTER the dependencies are fulfilled. It
SHOULD (imo) set the value of previous_orientation after each time the
check orientation process is finished.

So.. long story short: I think the previous_orientation=orientation
command should be taken out of the sequence above and placed as a
dependencie-free command behind the rest

Something like this: 406 if (previous_orientation == orientation /* only
rotate when stable */ && 407 orientation != screen_orientation &&
orientation != FLAT && 408 !orientation_lock) { 409
rotate_to(orientation); 410 } 411 previous_orientation = orientation;

I tried it with this code and if "run sequence once" - write
previous_orientation at the end after all other commands - "run sequence
again" - previous_orientation=orientation at the beginning - Hurray!

Greetings

Felix

— Reply to this email directly or view it on GitHub
#36.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBAgAGBQJVL8WtAAoJECjN6+QThfjzSFMH/2B2Dst2s2UtsHw2aqid/WE2
RvFAa7nuIj+3BkutoFJ5AfP8jcHwSf7ViL8yo64ytsOHjQhN1Tc68ejJZWKonCIS
JCIFJQDCRF/4Z1gtUDPoQJT+a8jZkX2dXuwy/y+acBf8XUlGh4zTscnGqHQUvjDT
f1he7/F+VBuqpS1iWkjYJbSzBqZBi8+0EA/cOvjozj/6GLzFQKIRzogRUm85mYgV
rf64mQwvhsn+ZJpXauOJG90dPTV2iSa95UaIvFHHa9bltyLiUsD4z84Z7zcFO3w5
CyevGNpwH7Ze5dSru27ADobBvN3qyhTBSS7FGHoc/iG5fjCDSJUrbPhE3cZ7qOo=
=yyez
-----END PGP SIGNATURE-----

@pfps
Copy link
Owner

pfps commented Apr 16, 2015

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Yes, your analysis is correct.

I wonder how that bug got into the code.

I made the change, and pushed it back to the repository.

Thanks for doing the hard work to figure out what was going wrong.

I also pushed a change to where some keyboard stuff is put, as Fedora 21
changed its location.

peter

On 04/16/2015 05:03 AM, z0mb0lix wrote:

Hey pfps.

I am almost certain I found a small error in your orientation.c code
that prevents the auto rotation:

406 if (previous_orientation == orientation /* only rotate when stable */
&& 407 orientation != screen_orientation && orientation != FLAT && 408
!orientation_lock) { 409 rotate_to(orientation); 410 previous_orientation
= orientation; 411 }

In line 406 the dependencies to execute the rotate_to - section are
defined as

if (previous_orientation == orientation /* only rotate when stable */ &&
orientation != screen_orientation && orientation != FLAT &&
!orientation_lock)

If I understand it correct, the value of previous_orientation is defined
as "-1" in the header section and gets a new value at two positions: 1)
after the dependencies above are met and rotate_to sequence is executed
(Line 409) it gets the same value as orientation, 2) during the
sigusr_callback_handler sequence (Line 225) it gets the same value as
screen_orientation.

BUT, here comes the clue: screen_orientation also only gets his value in
the rotate_to sequence. But that prevents the section from actually
running because the algorithms to set the values to fulfil the
dependencies are only executed AFTER the dependencies are fulfilled. It
SHOULD (imo) set the value of previous_orientation after each time the
check orientation process is finished.

So.. long story short: I think the previous_orientation=orientation
command should be taken out of the sequence above and placed as a
dependencie-free command behind the rest

Something like this: 406 if (previous_orientation == orientation /* only
rotate when stable */ && 407 orientation != screen_orientation &&
orientation != FLAT && 408 !orientation_lock) { 409
rotate_to(orientation); 410 } 411 previous_orientation = orientation;

I tried it with this code and if "run sequence once" - write
previous_orientation at the end after all other commands - "run sequence
again" - previous_orientation=orientation at the beginning - Hurray!

Greetings

Felix

— Reply to this email directly or view it on GitHub
#36.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBAgAGBQJVL+E6AAoJECjN6+QThfjzDakIAJve45eau9EvjgaMrOsWu5Vd
EhCyoh0El3zuounj7syXILanZBls6mMJScAkiN0MMAlHnvyD11HrVSTD34fY2Iv+
gH0OKm5mEbjASNtFexbGpfDNvtsUb0WLRw0rplKhgwrYsVDUhRMPbPFiaoYLrl14
b93kt6nlQ3oLqrcKJ8V6tKlK9pW+GyaKUSovU0czZnp/OnNl7Yviyo66Z93F+Cva
oDGRfMHpMXnS5coXDEnt09QTaiekVkofklRKFIlKeRmQ9PmtKhKpN6ca1EN5BEbn
l4jaRsJYQLfk0mPc/ZBtQn3r7Hc53FgNcLIEo7fpr+76NRlGIB0TxMlIHTBPiXg=
=n4EF
-----END PGP SIGNATURE-----

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

No branches or pull requests

2 participants