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

Activity log #6746

Merged
merged 20 commits into from
Aug 11, 2015
Merged

Activity log #6746

merged 20 commits into from
Aug 11, 2015

Conversation

Montellese
Copy link
Member

I know I'm a little late for the party but due to some issues in my media import work I came back to the activity log feature I proposed a while ago. Back then I didn't finish it because I didn't get much feedback from skinners so I've decided to do what's easiest.

The idea of the activity log is to have a place where a chronological list of activities that happened while Kodi was running can be stored. Most of the time I miss the kai toasts telling me that something happend. So it would be nice if there was a place where I could go and see all the "important" activities that have happened. That list would be sorted by datetime descending (i.e. the latest activity at the top) and would only show activities of the current instance (i.e. no persistance although that would be possible).

This adds the CActivityLog singleton which stores all activities (and can show some of them as kai toast notifications). Ideally most calls to CGUIDialogKaiToast would be replaced by calls to CActivityLog in the future. The feature set consists of

  • IActivity interface representing an activity consisting of a type, level, label, icon, description, details, date and possible action
  • CActivityLog to store and retrieve activites
  • CActivitiesDirectory supporting the activities:// protocol to retrieve list items representing activities
  • CGUIWindowActivityLog providing a window to display and manage activites
  • ActivityLog.xml for Confluence covering CGUIWindowActivityLog

I have added some basic activites like

  • installing, updating, enabling, disabling, breaking and uninstalling addons
  • media items that failed to be handled by the music/video info scanner

The most useful functionality of an activity is the fact that it can provide an action which can be executed when selecting the activity. As an example in case of an addon activity the user will be taken directly to the addon in the addon manager.

Here are two screenshots of the new functionality. The first is showing the available settings to enable/disable activity logging and to show the activity log window (this could certainly also be better integrated into another place in Confluence). The second is showing the activity log window (which is basically a combination of the full width list view and the select dialog details list view).

Activity log settings

Activity Log

A few things that are missing:

  • better/proper integration into Confluence
  • an icon to show in the top left corner of the activity log window
  • better integration of the different available levels (maybe with additional icons or colors for the activity list items?)
  • a dialog to show the details (not just label and description) of an activity. Right now there's no activity using details so there's no missing information.
  • more activity types

but IMO all of that can be improved later.

@Montellese Montellese added the Type: Feature non-breaking change which adds functionality label Mar 17, 2015
@Montellese
Copy link
Member Author

@mkortstiege: Can I ask you for an Xcode sync?

@mkortstiege
Copy link
Member

Sure, will do once #6747 is in so we do not have the unrelated changes in there (all the time).

@un1versal
Copy link
Contributor

Love the idea.

Small suggestion.

Would renaming the Debugging to Logging and create debugging section and activity section inside? and move this stuff there make more sense going forward?

If not anyway, I can have a go at creating a icon anyway for this.

@MilhouseVH
Copy link
Contributor

+1 on what @UniversaI said (both points, this is great). Icing on the cake would be persistence (complete or limited, maybe keep a rolling n days?), and accessing the contents of the activity log via JSON-RPC would be a "nice to have".

@Montellese
Copy link
Member Author

@universal: Yeah I'll give that a go. I didn't really wanna add an extra category for the activity stuff anyway.

@MilhouseVH: Yeah I wasn't sure if people really wanted persistance so I left it out for now. But obviously it can easily be added later.
Another nice to have thing would be integrating the activities with Android notifications (no clue if iOS has something similar as well). But since I wanted to try to get this into Isengard and it's kinda last minute I left out all the eyecandy stuff ;-)

@Hitcher
Copy link
Contributor

Hitcher commented Mar 17, 2015

As I posted on the forum could you use the kaitoast icons as indicators for each activity?

@MilhouseVH
Copy link
Contributor

Could the activity log be viewed as another panel in the System Information dialog (along with Hardware, Network etc.)?

@Hitcher
Copy link
Contributor

Hitcher commented Mar 17, 2015

RE: Using a dialog instead of window.

The buttons could easily be added to bottom of a dialog and the sort order button doesn't really seem necessary.

@MilhouseVH
Copy link
Contributor

Throwing this out there... should there be support for Profiles? For instance, logging when a new profile is loaded (or failed to load, invalid PIN etc.), created, deleted. Logging the current profile against which a particular activity took place might become relevant, depending on the activity. The profile corresponding to the activity would need to be listed against the activity, although maybe only if there is more than one profile configured for the client. Filtering by profile might also be useful.

@Montellese
Copy link
Member Author

Could the activity log be viewed as another panel in the System Information dialog (along with Hardware, Network etc.)?

It's available as a VFS so skins could integrate it anywhere using activities:// or activities://<level> or activities://<level>+. This is actually the reason why I added VFS support so that skins could show/integrate it wherever they want.

The buttons could easily be added to bottom of a dialog and the sort order button doesn't really seem necessary.

Sure, it simply was a lot more work (also integrating the context menu to be able to delete a specific activity etc) but I'm open to suggestions.

Throwing this out there... should there be support for Profiles? For instance, logging when a new profile is loaded (or failed to load, invalid PIN etc.), created, deleted. Logging the current profile against which a particular activity took place might become relevant, depending on the activity. The profile corresponding to the activity would need to be listed against the activity, although maybe only if there is more than one profile configured for the client. Filtering by profile might also be useful.

Ah I knew I forgot to mention something. Yes we should probably store the activities per profile and let the master user see all activites but all other profiles should only be able to see their activities.

@Memphiz
Copy link
Member

Memphiz commented Mar 17, 2015

Adding the activities to the Notification area of ios (or android) would really spam that area. Thats imo nothing we want. This area is more about getting push messages from messangers or calender entries or something like that.

@Montellese
Copy link
Member Author

@Memphiz: It could be limited to a certain level like only show warning and error or only error activities. There aren't that many activities right now and most of them are of level information.
TBH on android I already get a ton of notifications like one notification for every updated app. So getting a notification about having installed/updated an addon in Kodi wouldn't be that different.

EDIT: And obviously it would be optional and turned off by default.

@Memphiz
Copy link
Member

Memphiz commented Mar 17, 2015

Ok fine then for android - for ios its not the right type of messages to show in the notification area (as far as i can judge from the stuff that appears there normally).

@da-anda
Copy link
Member

da-anda commented Mar 17, 2015

nice. I'll think a bit about a nice UI integration. Important for me is that this doesn't end up becoming a devlog UI (seeing the "Kodi successfully started" entry which is superflous IMO - I'd only report startup issues and no success).
Do you have plans to support grouping of simmilar messages for some future version? Like f.e. "addon X updated" and "addon Y updated" could be grouped to something like "addons X, Y and Z updated" (so similar to what Android does).

As for the persistance. While this might be nice for a certain usecase (system log) I'd also like to suggest having some sort of "recent activities" that's more enduser oriented and is only showing "unread" activities one can dismiss/mark read. So again similar to what Android has.

@Montellese
Copy link
Member Author

@HitcherUK @MilhouseVH: What do you guys think about keeping the CGUIWindowActivityLog as a full-fledged window with all the features and add the possibility to open a simple dialog which would simply be the existing select dialog (maybe with some small extensions) showing the activities and allowing to activate them (without the functionality to delete/clear activities).

@da-anda: The "Kodi successfully started" activity is there for when we support persisting activities across multiple restarts in which case it is useful to know when kodi was shutdown/started.

For grouping we would have to increase the granularity of the activities. Right now there's simply an AddonManagementActivity for all activities involved with addons. For grouping all updates we'd have to introduce a separate AddonUpdateActivity which is obviously possible. But it will also make the UI more complex because you will have to be able to expand activities etc.

Marking activities as dismissed/read is currently supported by deleting them. On android you also can't get back the activities that you have dismissed. Unfortunately the deleting is not very well integrated yet i.e. you have to do it over the context menu. But you can clear all listed activities with a single button. "Recent activities" should also be possible by showing a list with a limited number of items which could be supported by the simpler dialog I proposed above. Or skinners could use the <content> tag (which already supports limiting) to provide their own skin integration. But obviously that's still untested as I don't really know much about skinning.

@da-anda
Copy link
Member

da-anda commented Mar 17, 2015

@Montellese with "dismiss" I didn't mean delete, because deleting would mean that the activity is removed from the "history" and this might not be wanted in case we persist (with persistence I see this more like an ever growing system log until you clear the log). So "dismiss" would just be some "mark as read" thing which would hide the entry from the "recent activities" dialog/window/whatever but you could still see the activity history in the dedicated window until you delete it there or flush the entire history. That's not required for an initial version ofc (unless you don't have better things to do ;) ) - just as an idea for future improvement.

About the grouping - what about subtypes specific to each main activity type? So we only have a AddonManagementActivity which can also be used to filter the activity log, but the activity could hold a subtype (integer/enum) and grouping may be applied on subtype level if activities occur within a certain timespan.

edit: this kind of grouping could only be done in a "recent activities" view and doesn't have to be applied on a probably huge history (once it's persisted)

@Montellese
Copy link
Member Author

@da-anda: Those are all potential features for the future and should all be possible. The current system is very basic but it already provides access to information that is otherwise well hidden in our log files.

@MilhouseVH: I've added basic support for storing activities per profile and it's only possible to view/delete/clear/interact with activities generated by the current profile. In the future it might be useful for the master profile to also see activities that took place in another profile which e.g. shares the same video database.

@mkortstiege
Copy link
Member

@Montellese please rebase and pick mkortstiege@9c6ccc3 (not compile tested - i am bit low on time right now but can do later today).

@MilhouseVH
Copy link
Contributor

@Montellese : Regarding the UI implementation, whatever you think is best - I'm not really up on all the different window types but might have a better idea once I see it (currently can't apply this PR to my test builds due to a strings.po conflict with #5747, might be nice to clear that one off the deck and rebase this one).

@Montellese Montellese force-pushed the activity_log branch 2 times, most recently from f8259ac to 7086f5d Compare March 17, 2015 14:26
@Montellese
Copy link
Member Author

@mkortstiege: Thanks, added.

I've merged the "activities" and "debug" settings into a new "logging" category.

@topfs2
Copy link
Contributor

topfs2 commented Mar 17, 2015

Regarding persistence, perhaps we should wait with that as a seperate PR. Perhaps we don't' even want it :) Also, I think persistence here would be a perfect opportunity to try out a NoSQL solution

@@ -12456,7 +12516,7 @@ msgstr ""

#: xbmc/addons/AddonInstaller.cpp
msgctxt "#24045"
msgid "Add-on does not have the correct structure"
msgid "Failed to install addon from zip file"

This comment was marked as spam.

@Montellese
Copy link
Member Author

@MartijnKaijser: Thanks for the "Add-on" reminder :-) There was another one as well and I fixed both.

@Montellese Montellese force-pushed the activity_log branch 2 times, most recently from 5ce470f to f44c2ef Compare March 18, 2015 09:11
@Montellese
Copy link
Member Author

@Memphiz: Thanks that would have been the next thing I'd have blindly tried. jenkins build this please

@Montellese
Copy link
Member Author

It worked! Thanks guys. Only some binary addons failed.

@Montellese
Copy link
Member Author

@wsnipex @Memphiz any cleanup needed for the buildsystem stuff or can I merge this?

@Memphiz
Copy link
Member

Memphiz commented Aug 11, 2015

i'm fine with it for osx/ios

@wsnipex
Copy link
Member

wsnipex commented Aug 11, 2015

fine for me as well. In the longer run, we should probably send a proper build system upstream, so we can get rid of the hacks.

@Montellese
Copy link
Member Author

I've already PRed an improved VS project structure and setup upstream and it go merged quickly.

@wsnipex
Copy link
Member

wsnipex commented Aug 11, 2015

nice, but VS is not enough. Currently, they have absolutely zero unix build sys wise.

@Montellese
Copy link
Member Author

Yes but my autoconf and pkg-config skills are nearly zero so I don't feel comfortable writing that and sending it upstream. So I only did VS where I felt confident enough.

@wsnipex
Copy link
Member

wsnipex commented Aug 11, 2015

I didn't imply you should do it, rather that I will when I find some time and patience.

Montellese added a commit that referenced this pull request Aug 11, 2015
@Montellese Montellese merged commit cee6a32 into xbmc:master Aug 11, 2015
@Montellese Montellese deleted the activity_log branch August 11, 2015 17:33
@Montellese Montellese added this to the Jarvis 16.0-alpha2 milestone Aug 11, 2015
@Montellese Montellese self-assigned this Aug 11, 2015
@MilhouseVH
Copy link
Contributor

Where is the best place to continue discussion about the Activity Log while the rough edges are knocked off? :) Maybe a dedicated forum thread?

This morning my repositories auto-updated, and I've now got 260+ notifications informing me about incompatible add-ons that I do not have installed. Surely we only need to be notified about problems with add-ons that we do have installed? Problems with add-ons that are not installed are likely to be of no interest to anyone.

screenshot

screenshot

Icons indicating the level (ie. Error, Warning, Info, Basic/Notify) would be useful - perhaps the "level" icon could appear to the left of the event/notification icon?

@Hitcher
Copy link
Contributor

Hitcher commented Aug 12, 2015

@MilhouseVH Here's the forum thread - http://forum.kodi.tv/showthread.php?tid=212067

@MilhouseVH
Copy link
Contributor

Thanks @HitcherUK - I'll post again there.

Edit: Actually, this looks like it might be a better discussion thread:

http://forum.kodi.tv/showthread.php?tid=211603&pid=2077605#pid2077605

@Hitcher
Copy link
Contributor

Hitcher commented Aug 12, 2015

Wasn't aware of that thread as I wasn't a member then ;)

@un1versal
Copy link
Contributor

@ronie
Copy link
Member

ronie commented Aug 14, 2015

@Montellese this somehow breaks a lot of addons in our repo.
nearly every skin is listed as incompatible and the same goes for 50% of the plugins.

http://xbmclogs.com/p7mwd6eth

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature non-breaking change which adds functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.