ActivityWatch / aw-watcher-window

Cross-platform window watcher (for use with ActivityWatch)
Mozilla Public License 2.0
83 stars 51 forks source link

fix: misc fixes to swift script on macos, incl URL inspection #69

Closed iloveitaly closed 1 year ago

iloveitaly commented 1 year ago

The new macos window watcher is working great so far! Will do some more testing over the next couple weeks.

iloveitaly commented 1 year ago

@ErikBjare @modderme123 mind taking a look at this one?

milomg commented 1 year ago

Looks good, although if you are removing the idle detection loop you might be able to remove the system idle time function as well

iloveitaly commented 1 year ago

Nice tip! Just removed in the latest commit.

@modderme123 do you recall what this comment is referring to?

TODO: set proper pulsetime

What's not right about the current pulsetime setup?

milomg commented 1 year ago

Here's how I understand pulsetime is used: https://docs.activitywatch.net/en/latest/_modules/aw_transform/heartbeats.html#heartbeat_reduce

This is very much intended for a polling system so that it doesn't ever need to diff anything, and can just send the latest value, and it will get merged if the pulsetime is long enough.

Because we intentionally don't poll, pulsetime doesn't make as much sense for us. On a new event, we just want to make sure the previous event duration is extended until the start of our new event, and the new event is created ASAP.

To extend the previous duration, we send the previous event again (except at the current time), with a pulsetime long enough that it gets merged. Then it doesn't really matter what the new event pulsetime is, because it shouldn't get merged (it necessarily has a different value, because our notifier was triggered).

I assume the original comment added by @ErikBjare was referring to wanting to switch partially over to a hyrbrid polling model like the chrome extension does (use a notifier for precise updates and then send them on a timeout, which seems like it accidentally loses resolution if you rapidly switch between apps, but will also automatically extend events even without switching between apps)...

iloveitaly commented 1 year ago

@modderme123 thanks for the comment, very helpful!

@ErikBjare this is ready for review and is working great for me locally. The main changes here are:

ErikBjare commented 1 year ago

@iloveitaly Waiting for you to address @modderme123's comments :)

iloveitaly commented 1 year ago

@ErikBjare @modderme123 comments addressed!

ErikBjare commented 1 year ago

@iloveitaly Before merging this, does this resolve https://github.com/ActivityWatch/activitywatch/issues/787?

iloveitaly commented 1 year ago

@ErikBjare I think so, although I wasn't running into the identical issue he experienced. My window bucket was empty, but I had no issues with accessibility permissions. I've been running this for a couple weeks at this point and my window activity is back to normal on macos.

ErikBjare commented 1 year ago

Alright, I tested it and it worked for both Chrome and Safari.

However, I decided to dig a bit deeper, and removed the Automation permissions for my terminal (Alacritty). After that, titles and URLs were blank again.

At minimum, it feels like we should at least get a window title even without Automation permissions, right?

Screenshot 2022-10-26 at 16 42 17

(took the screenshot after I enabled again, at which point it started working again)

ErikBjare commented 1 year ago

Not at my Macbook, so can't test it myself, but going to eagerly assume this is good to merge.

Thanks again for the contributions @iloveitaly! Sorry for the slow merge :)