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

Update system-arjs.js / Aframe Build / Exponential Backoff #304

Merged
merged 1 commit into from
Oct 16, 2021

Conversation

qbytx
Copy link

@qbytx qbytx commented Jul 3, 2021

Added exponential backoff function to the Aframe build (system-arjs.js)

⚠️ All PRs have to be done versus 'dev' branch, so be aware of that, or we'll close your issue ⚠️

What kind of change does this PR introduce?
I have added a simple exponential backoff function to the Aframe build (system-arjs.js)

Can it be referenced to an Issue? If so what is the issue # ?
No, this PR was not made in response to an issue.

How can we test it?

If you build/make the Aframe version of ar.js and are able to use it, it should be working correctly. The previous build used a setInterval function to resize the ar.js document element - this setInterval function fired every 33.3 seconds, and had a maximum duration of several hours. The function I made to replace it will fire at 33.3 millis, 66.6 millis, 133.2 millis, etc --- doubling every time, until a limit is hit (by default, 1 second, as noted in the comments requesting this feature)

It also has a maximum duration setting, but by default it will run forever (duration is set to Infinity by default).

Summary
I saw in the comments that an exponential backoff function was intended, but not yet written - So I made one (setBackoff). I tested the build and it works (more info below). Much like setTimeout & setInterval, setBackoff takes a function and a duration. The duration is set to Infinity by default, meaning the code will continue to repeat without limit. If a duration is provided, it will be called until that duration is exceeded. The final optional parameter sets the limit or ceiling for the backoff in milliseconds. The default value is 1000 (1 second), meaning the greatest interval of the backoff will not exceed 1 second.

Does this PR introduce a breaking change?
No, this change requires no refactoring.

Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser

To test, I created a new build of aframe-ar.js that included my changes. I then used the new build in one of the example projects for Aframe (ar.js/aframe/examples/marker-based/basic.html).

There were no errors in the web console, and I was able to make the TREX model appear over the hiro marker, as expected. I did this both on my machine (win10, using a webcam) and my device (android chrome).

Tested on Windows 10 Home version 20H2, 64-bit operating system, x64-based processor (Browser: Firefox 89.0.2)
Tested on Pixel 2, Android 11 (Browser: Chrome 91.0.4472.120)

Other information
N/A

Added exponential backoff function to Aframe build (system-arjs.js)
@qbytx qbytx changed the title Update system-arjs.js Update system-arjs.js / Aframe Build / Exponential Backoff Jul 3, 2021
@qbytx
Copy link
Author

qbytx commented Jul 4, 2021

also, I could submit a different pull request that is more cleaned up - or run more tests if needed

@kalwalt kalwalt self-assigned this Oct 12, 2021
@kalwalt kalwalt added the enhancement New feature or request label Oct 12, 2021
@kalwalt
Copy link
Member

kalwalt commented Oct 12, 2021

Thank you @qbytt for this! It's a nice improvement, i will test this, @nickw1 what do you think about?

@kalwalt
Copy link
Member

kalwalt commented Oct 12, 2021

Just one note: you haven't rebuild the libs. You should run the makefile with the command make to rebuild the aframe libs.

@nickw1
Copy link
Collaborator

nickw1 commented Oct 16, 2021

Thank you @qbytt for this! It's a nice improvement, i will test this, @nickw1 what do you think about?

@kalwalt seems fine to me.

@kalwalt kalwalt merged commit 6a6a9f4 into AR-js-org:dev Oct 16, 2021
kalwalt added a commit that referenced this pull request Nov 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants