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

fixed zero-duration bug #40

Merged
merged 4 commits into from
Jun 29, 2020

Conversation

nicolae-stroncea
Copy link
Member

Fixes #34, #39, and parts of ActivityWatch/activitywatch/issues/440

If event is AFK(screen_non_interactive), it is stored as queuedAfkEvent, and you move on to next item. On next item, if the app name of current event matches event before queuedAfkEvent, then you send new item first, and then the afk event (to allow them to be merged as 1 event by AW), otherwise if they have different names, send queuedAfkEvent first, and then current event

@nicolae-stroncea
Copy link
Member Author

I kept getting errors when compiling Rust, so I wasn't able to send/retrieve events from the server, I just built everything except for web-ui, and aw-server. I tested everything by logging it in the sendHeartbeatHelper method which acted as sending it to Rust.

Therefore, this section needs some extra testing, as I'm not sure if it retrieves the name correctly.

val lastEvent = getLastEvent()
if(lastEvent != null) {
    var prevEventData = JSONObject(lastEvent.getString("data"))
    prevEventAppName = prevEventData.getString("app")
    Log.w(TAG, "lastAppName: ${prevEventAppName}")
}

@johan-bjareholt
Copy link
Member

I kept getting errors when compiling Rust, so I wasn't able to send/retrieve events from the server

What were the errors? The only thing needed should be a somewhat recent version of rust nightly.

@ErikBjare
Copy link
Member

Saw this just now after my comment in #39.

This seems like a significant complication of the logic, which feels like it shouldn't be needed. As I asked in #39, do we need the SCREEN_(NON_)INTERACTIVE events at all? That would be a much easier fix.

@nicolae-stroncea
Copy link
Member Author

nicolae-stroncea commented Jun 23, 2020

@ErikBjare as per the discussion in #39, I removed SCREEN_(NON)_INTERACTIVE. Tested it, and it behaves correctly. I reverted the previous commits and added the changes on top in order to do it in the same PR but let me know if you prefer I open a new PR with a clean commit history instead

@nicolae-stroncea
Copy link
Member Author

@johan-bjareholt It was either rusqlite, or ring. I tried it again today and it compiled without issues, likely due to the dependancy updates on June 16/17

Copy link
Member

@johan-bjareholt johan-bjareholt left a comment

Choose a reason for hiding this comment

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

Looks good to me,
@ErikBjare what do you think? You know more about this code than I.

Copy link
Member

@ErikBjare ErikBjare left a comment

Choose a reason for hiding this comment

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

I think this looks good. Although there might still be issues as discussed in #39 I think we should go ahead and merge this, and then we can compare the stats with SmarterTime/Digital Wellbeing to see how severe the remaining issues are.

Just waiting for Travis to complete and then I'll merge.

@ErikBjare
Copy link
Member

@johan-bjareholt Looks like Travis fails when building aw-server-rust, do you have any idea about that?

   Compiling rocket_http v0.4.5
error: proc macro panicked
   --> /home/travis/.cargo/registry/src/github.com-1ecc6299db9ec823/rocket_http-0.4.5/src/parse/uri/parser.rs:119:34
    |
119 |             let path_and_query = pear_try!(path_and_query(is_pchar));
    |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: message: called `Option::unwrap()` on a `None` value
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
error: aborting due to previous error
error: could not compile `rocket_http`.

@johan-bjareholt
Copy link
Member

@ErikBjare I have no idea why, works in the aw-server-rust repo and aw-android seems to run a quite recent version of it.

On a side note the commit log in this PR looks kind of odd, you should probably squash them or something.

@nicolae-stroncea nicolae-stroncea force-pushed the fix_zero_duration branch 2 times, most recently from de9fe91 to c18ebd3 Compare June 27, 2020 20:50
@nicolae-stroncea
Copy link
Member Author

Just fixed the commit log. I kept the initial fix in the history of the PR because the approach from it might be useful later, especially since there are still some issues(though much smaller scale now) as discussed in #39

@ErikBjare ErikBjare force-pushed the fix_zero_duration branch from c18ebd3 to bbe44e2 Compare June 29, 2020 19:44
@ErikBjare
Copy link
Member

ErikBjare commented Jun 29, 2020

Rebased on top of master, hoping that fixes Travis.

@ErikBjare
Copy link
Member

It didn't, looks like it could be related to rust-lang/rust#73293

I'll update the dependencies in aw-server-rust and try again.

ErikBjare added a commit to ActivityWatch/aw-server-rust that referenced this pull request Jun 29, 2020
@ErikBjare
Copy link
Member

ErikBjare commented Jun 29, 2020

Managed to reproduce on latest nightly and fixed by:

cargo update -p pear
cargo update -p pear_codegen

@ErikBjare
Copy link
Member

Yay, CI passing. Merging.

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

Successfully merging this pull request may close these issues.

Many events with a duration of 0
3 participants