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

WIP: Calendar / Timeline app and service #790

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Zorvalt
Copy link
Contributor

@Zorvalt Zorvalt commented Oct 28, 2021

This PR is a proof-of-concept for a calendar/timeline app and its service. It will be split in smaller PRs when the maturity of the PR allows it.

HELP WANTED ! Especially for the design

Current state (based on InfiniTime 0.14.1) :

  • Working calendar service (GadgetBridge sync protocol)
  • Working ugly week overview
  • Working ugly timeline of events

Although there is a working base, the direction is not fixed. Do not hesitate to suggest fundamental changes or start brainstorms :-)

More information on the fork wiki : https://github.com/Zorvalt/InfiniTime/wiki/Calendar-and-timeline:-first-ideas-and-implementation

@Avamander Avamander mentioned this pull request Dec 4, 2021
11 tasks
@Avamander
Copy link
Collaborator

Avamander commented Dec 4, 2021

To continue our conversation, I would propose that we think of how to patch what is currently #353 into accepting calendar events. I would wait for it to be finalized first though.

It's not technically difficult, but we would have to move a few things around and figure out what is a "calendar event". What data is needed, what can be optional? What might people use, if we make it possible?

@Avamander Avamander self-requested a review December 4, 2021 12:21
@Zorvalt
Copy link
Contributor Author

Zorvalt commented Dec 4, 2021

I would wait for it to be finalized first though.

Fine by me

It's not technically difficult, but we would have to move a few things around and figure out what is a "calendar event". What data is needed, what can be optional? What might people use, if we make it possible?

Here are a few thoughts...

Bare minimum

The bare minimum to display and sync a calendar consists of :

  • Title
  • Date start
  • Optional Date end (or duration)
  • ID for the companion app to be able to update/delete events

iCalendar format

We can look at existing standards. I think the obvious one is iCalendar as it is probably the most popular and will probably work with a lot of calendar apps. So we can either support all of iCalendar fields or a subset. At least, if we use the same names and format when possible, it will probably reduce the friction with companion apps.

A big advantage to using iCalendar is that it paves the way for the other types. I am sure users will want InfiniTime to handle todos at some time...

iCalendar events can be really big. If we decide to follow that path, we will probably need to specify some limitations. I do not think a user expects its watch to have its memory filled by a calendar description.

iCalendar components and their properties
wikipedia: iCalendar components and their properties - RFC 2445

Watch-generated events

Should the companion app be able to extract/display watch-generated events in its calendar?

Multiple calendars

Support for multiple calendars? I use different calendars to separate contexts, I think it would be useful to be able to display events in different colors.

@Zorvalt
Copy link
Contributor Author

Zorvalt commented Mar 8, 2022

New look with event colors possibilites :
timetable_debug_light

@Zorvalt
Copy link
Contributor Author

Zorvalt commented Mar 8, 2022

Now that this part is done (tested with the Gadgetbridge fork and it works), let's focus on the core : a generic timeline manager for all event types and all apps on the watch.

@Avamander I dived deeper in your code now in order to merge what we have in common a code a first draft of generic timeline. I have a few questions to clarify my understanding, if you have the time.

  1. The expiration field from the TimelineHeader class in WeatherData.h

    1. Is this field really necessary? If I understand your goal correctly, we could either discard the event when it's timestamp is passed or have a global setting specifying how long to keep events. This information might also be hard to define and I think most of the time (if other types of events are implemented) it will be hard-coded to some arbitrary value, probably to match the timestamp.

    2. In the comment above, you wrote:

      If there's a newer event of the same type then it overrides this one, even if it hasn't expired.

      I am not sure I understand: does this mean that there cannot be, for example, two “wind” events in the weather? If so, what about forecast?

    3. In the calendar events, I have the duration field which is close in terms of final behavior (if I understood correctly). Do you think we can substitute the expiration field with a duration one and handle the expiration in the code, based on the event type?

  2. The calendar synchronization I implemented follows the Pebble protocol from Gadgetbridge. The companion app is responsible for telling which event to add or delete (except for expired events, the watch can delete them). This is especially useful since the phone(or other companion) calendar is the source of the watch events. I think this behavior might be a part of the controlcodes class in WeatherData.h

    1. What about weather events, do you think a weather app on the phone should be able to dictate the addition/deletion of forecast events? If this behavior is generic enough, I can move it from the calendar logic to the timeline logic and provide a single timeline BLE service to which sub-services (calendar, weather, etc.) will subscribe to receive their events.
    2. To prevent events being deleted by the wrong authority (e.g. DelTimeline control code) I imagine, we could also have a source field in the header to indicate what authority the companion app has ; maybe with two values local/companion. This prevents the phone from wiping everything, including local events(as proposed in a comment above). What do you think ?

@Avamander
Copy link
Collaborator

Avamander commented Mar 8, 2022

If I understand your goal correctly, we could either discard the event when it's timestamp is passed or have a global setting specifying how long to keep events.

Yes, but that assumes that an event is valid from the start of it being pushed to the watch. It might be that an event is valid only for an hour tomorrow. For example currently it's something like Wind is 6 m/s starting at 1646748015 for 3600 seconds.

This information might also be hard to define and I think most of the time (if other types of events are implemented) it will be hard-coded to some arbitrary value, probably to match the timestamp.

It has to follow some kind of reasonable precision. E.g. weather is not the same for a week, or for a second.

I am not sure I understand: does this mean that there cannot be, for example, two “wind” events in the weather? If so, what about forecast?

Say we have two Wind that are valid at the same time.

a) Wind is 6 m/s starting at 1646748015 for 3600 seconds.
b) Wind is 2 m/s starting at 1646740015 for 42800 seconds.

Then the most recent one a) should be picked for use before the second one. This is basically only formalizing how overlapping data should be interpreted, not that there may not be any.

In the calendar events, I have the duration field which is close in terms of final behavior (if I understood correctly). Do you think we can substitute the expiration field with a duration one and handle the expiration in the code, based on the event type?

This actually might be more complex, because you might not wish to clean up an event from the timeline the moment it ends. So expires and duration are two different things, expires is really for marking how long data is valid.

What about weather events, do you think a weather app on the phone should be able to dictate the addition/deletion of forecast events.

Yes. That is on the current roadmap. To have wipe and deletion.

To prevent events being deleted by the wrong authority (e.g. DelTimeline control code) I imagine, we could also have a source field in the header to indicate what authority the companion app has ; maybe with two values local/companion. This prevents the phone from wiping everything, including local events(as proposed in a comment above). What do you think ?

That is very difficult. I would just caution companions to wipe things they can't replace.

@Zorvalt
Copy link
Contributor Author

Zorvalt commented Mar 8, 2022

It is now possible to test this feature using my fork of InfiniSim. Just run :

git clone --recursive -b timeline_integration https://github.com/Zorvalt/InfiniSim.git

@Zorvalt
Copy link
Contributor Author

Zorvalt commented Mar 8, 2022

@Avamander Thank you for the feedback

In the calendar events, I have the duration field which is close in terms of final behavior (if I understood correctly). Do you think we can substitute the expiration field with a duration one and handle the expiration in the code, based on the event type?

This actually might be more complex, because you might not wish to clean up an event from the timeline the moment it ends. So expires and duration are two different things, expires is really for marking how long data is valid.

I agree with your argument but I am not convinced it represents a significant part of the possible event types. I think that "most of the time"(TM), expiration will mean duration and the real expiration depends on the application. For example, Gadgetbridge always syncs the events for the next 7 days. As the timetable developer, I can choose if I want to keep expired events for the rest of the day or not. I think that this reasoning can apply to most apps. In the rare cases where an expiration timestamp is needed to indicate that the event must be destroyed, it can be added as a sub-field in the event itself.
If I understood correctly, it also works for weather events : the current expiration field is indeed a kind of duration and the weather apps can choose how long to keep them after the event is expired.
All in all, keeping only the duration field keeps the idea of event being expired and allows us to add a strict expiration field for event types that needs it.

Yes. That is on the current roadmap. To have wipe and deletion.

Ok so I'll move that in the generic timeline code

That is very difficult. I would just caution companions to wipe things they can't replace.

It sounded less complicated in my mind but reading it again now, I agree with you.

@Avamander
Copy link
Collaborator

Avamander commented Mar 8, 2022

I agree with your argument but I am not convinced it represents a significant part of the possible event types. I think that "most of the time"(TM), expiration will mean duration and the real expiration depends on the application. For example, Gadgetbridge always syncs the events for the next 7 days. As the timetable developer, I can choose if I want to keep expired events for the rest of the day or not. I think that this reasoning can apply to most apps. In the rare cases where an expiration timestamp is needed to indicate that the event must be destroyed, it can be added as a sub-field in the event itself.

I guess this could be written down. "Field expires is seconds after this event ends. It will be wiped from the device either on OOM or when it doesn't make sense to keep or a maximum of 24h of expiricy. Expired events might be displayed as historical data."

@Riksu9000 Riksu9000 added the new app This thread is about a new app label May 2, 2022
@devnoname120
Copy link

@Zorvalt Friendly bump 🙏

@Zorvalt
Copy link
Contributor Author

Zorvalt commented Jun 18, 2022

@Zorvalt Friendly bump pray

Hi @devnoname120, I will have some time this summer.

It is probably as frustrating for me as it is for you. I initially developed this because I need it on my watch so I promise I want this to be merged. I just lack the time :-/

As the feature has been "functional" for about two years now, I hope this year is the one. I will at least rebase it on develop and we'll see if I have enough time to get the generic timeline to work.

I plan to start working on this in July.

To anyone reading this. I am still not the best designer in the world (see the picture above, haha). Any suggestion, even a quick drawing on the corner of a piece of paper is welcomed :-)

@devnoname120
Copy link

devnoname120 commented Jun 18, 2022

@Zorvalt I want to implement something like MeetingBar but for InfiniTime in a watchface:
image

I'll probably use your calendar service as a base.

@Zorvalt
Copy link
Contributor Author

Zorvalt commented Jun 18, 2022

@devnoname120 The idea is to integrate events inside a more generic "timeline" based on what @Avamander did for the weather app. That's the reason it's taking so long to be merged. So I think you will need to read events from there, in the end.

Your idea is good and I would greatly appreciate such a feature. More incentives for me to work on this PR ;-)

There already are a few helpers such as GetTodayMaxTemp(). Your use-case is great as it allows to think about how applications should access timeline objects. I think we can boil it down to two possiblities : either add another helper (maybe GetNextCalendarEvent()) or expose a generic way to browse events (maybe an iterator like in the CalendarManager and possibly with an event type filtering option). Both have pros and cons... What do you think? What would you prefer?

@devnoname120
Copy link

@Zorvalt Thank you for your thoughtful reply.

I think I'll go for the easiest solution at first: iterate through CalendarManager::CalendarEventIterator and return either an event that is currently happening or else the closest event in the future.

How many events should I expect CalendarManager::CalendarEventIterator to contain?
This approach may or may not be performant enough depending on that.

It's likely that it would be OK though as I would only need to reiterate when the calendar is updated. Otherwise I can just update the current countdown from the current event entry and current time.

Once the timeline is ready, I could switch to it to make the code more intuitive & improve performances (assuming your timeline data structure is sorted by start or end datetime).


What would you think of splitting up this PR so as to only add the Calendar service in the first PR?
It would help untangle the development of the calendar apps and my ‶next event″ feature.

@Avamander
Copy link
Collaborator

expose a generic way to browse events (possibly with an event type filtering option). Both have pros and cons... What do you think? What would you prefer?

In theory the function already exists in Weather, it just expects to filter weather-related timeline item types. I haven't had a need to test it very throughly though. But it would work.

How many events should I expect CalendarManager::CalendarEventIterator to contain?

I'd say 5 events is sufficient, more can be synced on-demand later on, RAM is really quite tight already.

assuming your timeline data structure is sorted by start or end datetime

Can't be assumed with Weather and it's intentional. Iterating is quite fast with so few elements and demands vary immensely (do I want what's to come, what is or what was?).

What would you think of splitting up this PR so as to only add the Calendar service in the first PR?

This would work.

@devnoname120
Copy link

@Avamander Any updates on this? 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new app This thread is about a new app
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants