*** zbenjamin is now known as Guest32359 | 02:03 | |
*** zbenjamin_ is now known as zbenjamin | 02:03 | |
*** frinring_ is now known as frinring | 06:43 | |
dcaliste | Hi chriadam, how are you ? | 07:58 |
---|---|---|
chriadam | hello dcaliste, I'm well thanks - how are you? | 07:59 |
dcaliste | I'm fine thank you. I've seen you started to migrate some of the calendar code to upstream KCalendarCore. What do you think of it ? | 08:02 |
dcaliste | I've made some comments in your WIP for Google sync plugin. | 08:02 |
chriadam | I have started, yes - lots more work to do yet, as so far I've just done "mechanical" passes, without thinking too much about things :-) | 08:02 |
chriadam | I saw your comments - thanks for that - I haven't yet had time to digest those | 08:03 |
chriadam | the "all day" one needing a separate flag seems fine | 08:03 |
chriadam | but I still wonder about the ClockTime one | 08:03 |
chriadam | e.g. if I am a creature of habit, and want to have lunch at exactly 12:30 pm every day, no matter which country I am in | 08:03 |
chriadam | what do we do in that case? in the past, I'd have expected us to have saved an event with a dtStart of 12:30 in ClockTime spec | 08:04 |
chriadam | whereas now, I think it might get saved as LocalTime (i.e. whatever the system time zone is, when I save the event), and then I would expect that it might get transformed to different time when I travel to a different tz | 08:04 |
chriadam | but I might be mistaken / misunderstanding | 08:04 |
dcaliste | This case is the simple one, in my opinion. Old ClockTime spec converts to LocalTime, mechanically. | 08:04 |
dcaliste | Then, Qt when doing time comparison for a LocalTime spec is always using the system time zone, whatever zone you are in. | 08:05 |
dcaliste | I mean that if your in Paris zone, it will say 11:30 UTC. | 08:05 |
dcaliste | But you travel to Vietnam, the same event will convert to 6:30UTC. | 08:06 |
chriadam | what do you mean by "the event will convert" in this case? (e.g. the event I previously saved?) | 08:07 |
dcaliste | I mean, that a QDateTime(QDate(...), QTime(12,30), LocalTime).toUtc() will give this. | 08:07 |
chriadam | ok - interesting, so it's basically identical in semantics to the old KDateTime ClockTime spec? | 08:08 |
dcaliste | Then, in mKCal, this event is saved *without* the timezone info as a floating date time, so it is reread as a LocalTime spec. | 08:08 |
chriadam | right | 08:08 |
chriadam | ok, nice. | 08:08 |
dcaliste | Yes, this one can be convert mechanically from ClockTime to LocalTime. | 08:09 |
chriadam | so what about when I _do_ want to specify a specific timezone for the event (e.g. call with my mother every Saturday at 7pm in my current local time zone (which happens to be AEST = GMT+10)) -> in this case when I travel to New Zealand I want the event to be not LocalTime but still AEST/GMT+10 etc | 08:10 |
chriadam | I guess I will just have to double check how we use LocalTime in the code, and ensure that we use LocalTime for those cases which are meant to be ClockTime, but use some specific non-LocalTime timespec for the other cases? | 08:10 |
dcaliste | The one that needs to look at is the LocalZone old spec. 99% of the time is will convert to a TimeZone spec with the systemtime zone. But I think that 1% of the cases, it has been misused as a ClockTime meaning. | 08:11 |
dcaliste | For your example, you create a QDateTime(..., QTimeZone::systemTimeZone()) and you will be fine. | 08:11 |
dcaliste | mKCal will save the time zone and restore it or read. So if you travel to another timezone, the time will still be the same in absolute but will be a different one on your clock. | 08:12 |
dcaliste | That's why I think the LocalZone spec was removed from QDateTime, because it is redundant with the TimeZone spec and misleading with the ClockTime because does it mean "time zone at creation time" or "always system time zone". | 08:13 |
dcaliste | And I think that's what we need to check during the migration because 99% of the cases, the code is written with the LocalZone spec. | 08:14 |
dcaliste | Up to us to know if it means "time zone at creation time" -> TimeZone spec or "always system time zone" "always system time zone" -> LocalTime. | 08:15 |
chriadam | that makes sense. | 08:17 |
chriadam | thank you for the in-depth explanation | 08:17 |
chriadam | I am now certain that I did some of those wrongly at least in my sailfish-eas migration so far | 08:17 |
dcaliste | It took me litteraly years to get this ;) | 08:17 |
dcaliste | But once I grasp the idea, it looks quite simple and logical. | 08:18 |
dcaliste | About the migration, I noticed you worked on nemo-qml-plugin-calendar, but I did it also : https://git.sailfishos.org/mer-core/nemo-qml-plugin-calendar/merge_requests/38 | 08:18 |
dcaliste | But I must admit that I need to redo it anyway, because it was done as a first attempt a long time ago when these spec considerations were totally unclear to me. | 08:19 |
chriadam | yeah I started looking at that one but it didn't even rebase cleanly | 08:19 |
dcaliste | So don't take this one as an example yet :/ | 08:19 |
chriadam | and I figured it would probably be quicker for me to just do it from scratch | 08:20 |
chriadam | again, I just did a mechanical pass so entirely likely that I messed things up | 08:20 |
chriadam | but it builds, at least. shrug. | 08:20 |
chriadam | just to double check that I understood correctly: | 08:22 |
chriadam | 1) if I want an event like "lunch is at 12:30 pm every day, regardless of current timezone" then that is QDateTime(date, 12:30, Qt::LocalTime); | 08:23 |
chriadam | 2) if I want an event like "call my mother at 7pm (current timezone, AEST)" then that is QDateTime(date, 19:00, QTimeZone::systemTimeZone()) | 08:23 |
chriadam | ? | 08:23 |
dcaliste | Totally Exact. | 08:23 |
chriadam | great, thanks. | 08:23 |
chriadam | another thing I noticed: some methods to parse timezone info are now not exposed by kcalcore | 08:24 |
dcaliste | Yeh, another colateral damage to the migration : KCalendarCore does not allow user defined timezone anymore. | 08:25 |
dcaliste | They just allow the system ones. | 08:25 |
chriadam | specifically, relating to (windows/eas) TIME_ZONE_INFORMATION struct stuff. it's not a problem -> I've implemented some parsing and conversion stuff in sailfish-eas now. but it's slightly different | 08:25 |
chriadam | yes, like you say -> no longer a "custom" timezone, but instead, I now try to "detect" which QTimeZone is "closest" and use that | 08:25 |
dcaliste | Normally KCalendarCore still do this internally, in this way if I remember correctly: | 08:26 |
chriadam | my conversion / detection code is a bit funky because the exchange tz struct only supports a single transition (i.e. standard + daylight), whereas QTimeZone supports arbitrarily many transitions etc. | 08:26 |
dcaliste | - if the system has a matching name with a system one, the system one override the user defined time zone. | 08:27 |
dcaliste | - if no time zone in the system is matching the name, then the user defined time zone is parsed and its definition is compared to all system time zones. if one matchs, it is returned. | 08:27 |
dcaliste | - if none match, a warning is raised (or an error ?). | 08:28 |
chriadam | is there some API to access that? what do I pass in, a VTIMEZONE component? or? | 08:28 |
dcaliste | So you can still have a VTIMEZONE ical entry, but has long as its name is in the system, the content is ignored. | 08:28 |
dcaliste | Yes, you can create an arbitrary calendar with this VTIMEZONE entry and see which time zone is returned for your calendar. | 08:29 |
chriadam | in my case, I would have to build a VTIMEZONE component and fill it out with the transition info described in the TIME_ZONE_INFORMATION struct, I guess. but better to let kcalcore do it once (properly) than me to reimplement the "detection" logic, I think. | 08:29 |
dcaliste | Parse the forged ICS calendar with the usual icalformat methods from KCalendarCore. | 08:29 |
chriadam | thanks, I will investigate | 08:30 |
dcaliste | They try to match the windows names also if I remember correctly, let me give you the line number... | 08:30 |
dcaliste | https://invent.kde.org/frameworks/kcalendarcore/-/blob/master/src/icaltimezones.cpp#L510 | 08:31 |
dcaliste | This method is using the icalTz.id to see if QTimeZone knows about this name. | 08:32 |
dcaliste | Then tries with windowsIdToDefaultIanaId(). | 08:32 |
chriadam | if both of those fail (e.g. because the names we receive from this struct are not "proper" windows tz id values, but instead some display name / old identifier due to version of activesync we support, then it will fall down to the bottom loop | 08:35 |
chriadam | in that case, it will fill out the icalTz.standard+daylight values via parsePhase() | 08:35 |
dcaliste | Yes, and then the match is done in the parent in https://invent.kde.org/frameworks/kcalendarcore/-/blob/master/src/icaltimezones.cpp#L433 | 08:35 |
chriadam | my brain is too small. that resolveICalTimeZone() method seems strange to me | 08:38 |
chriadam | ++matchedTransitions; --> but I don't understand how the match was determined. it never compares the offsets | 08:39 |
chriadam | oh, wait, it only compares timezones which match the phase.utcOffset. never mind. | 08:39 |
chriadam | hmm. it checks icalZone.standard, but ignores the icalZone.daylight ? I must be misunderstanding | 08:40 |
dcaliste | Never read it in details myself, just imagined it should try to match the user defined transition with the known ones from the system... | 08:40 |
dcaliste | I think it's very seldom used in KDE case, because most names are standard anyway and available on the system. So it may be buggy. | 08:41 |
chriadam | more likely I'm just misunderstanding | 08:42 |
chriadam | there is a: const auto &transition = *it; there -> now that transition struct might contain the offsets of what it is transitioning to, not just the date of the transition | 08:42 |
chriadam | but without knowing the type of "transition" there (because they use auto) I can't ... immediately tell from reading the code | 08:43 |
dcaliste | Isn't candidate.transitions contaning both standard and daylight saving ones ? | 08:43 |
dcaliste | I don't know what the abbreviation means in that context. | 08:43 |
dcaliste | Maybe it contains the type information also... | 08:44 |
chriadam | well, I trust it more than I trust my own hand-rolled code. and if something starts to fail, I will look deeper. | 08:44 |
chriadam | I will just need to port the unit test we had in mkcal, and put it in sailfish-eas instead (i.e. to test that the mstz definition is handled properly when creating the mkcal event) | 08:44 |
chriadam | all good :-) | 08:45 |
chriadam | thanks for pointing out that method, I will definitely use that instead, to reduce code duplication etc. | 08:45 |
dcaliste | But I agree, that for the provided ical time zone, it is only using the standard ? | 08:45 |
dcaliste | standard phase. | 08:45 |
dcaliste | Strange indeed. But as I said, I suspect this not to be very tested... I may be wrong. | 08:46 |
chriadam | well, a good chance for me to provide some testing for it | 08:46 |
chriadam | if there's one thing I know about myself, it's that: when it comes to timezone-related code, I trust other people more than me haha. | 08:47 |
chriadam | and also "unit tests trump intuition" haha. | 08:47 |
dcaliste | Yeh, exactly. And if some others can take the burden of maintainance for this (with a little help), that's even better ;) | 08:49 |
chriadam | definitely | 08:49 |
dcaliste | What about the possibility to merge the mKCal ExtendedCalendar rewrite ? | 08:50 |
chriadam | next topic: I had a quick look at your caldav+buteo-syncfw PRs. not in depth, and didn't do any on-device testing, even though I promised that I would (unfortunately I got sidetracked with a high-priority contacts sync bug which I had to fix) | 08:50 |
chriadam | but from my quick look, it certainly seems good to me. | 08:50 |
dcaliste | Great, thank you. | 08:51 |
chriadam | regarding the mkcal extendedcalendar thing: basically, I want to avoid merging anything to devel until after Thursday (branching) occurs | 08:51 |
chriadam | currently, the only things we're merging is bugfixes for known issues | 08:51 |
chriadam | to avoid destabilising devel att he moment | 08:51 |
dcaliste | Ok, I see. No problem. | 08:51 |
chriadam | once the branching is done, I will start merging a variety of your PRs including that one | 08:52 |
chriadam | most likely monday next week, just in case something with branching needs attention over the weekend etc. | 08:52 |
dcaliste | About bug fixing, what about this one : jolla-calendar#278 ? | 08:52 |
chriadam | let me check | 08:52 |
dcaliste | Not of the highest importance :) | 08:53 |
chriadam | LGTM - have poked Pekka but don't see any reason why we shouldn't merge that one | 08:53 |
chriadam | thanks very much | 08:53 |
dcaliste | Thanks. | 08:53 |
dcaliste | About the mKCal ExtendedCalendar rewrite, we wondered last week about the discrepancy in rawEvent() behaviour between MemoryCalendar and ExtendedCalendar. | 08:54 |
dcaliste | I've looked in nemo bindings, and it's used only in one place. | 08:55 |
dcaliste | Before a for loop on returned events. It could be nice to put the visibility switch there and drop the specificity in ExtendedCalendar, couldn't it ? | 08:55 |
dcaliste | See https://git.sailfishos.org/mer-core/nemo-qml-plugin-calendar/merge_requests/66 | 08:56 |
chriadam | nice! that feels far nicer | 08:57 |
chriadam | than having an override method doing something different | 08:58 |
dcaliste | The remaining question if you agree for this, is : "is raw*() used outside the nemo bindings ?" | 08:58 |
chriadam | let me check | 08:59 |
chriadam | at least, grep for rawEv returns nothing in sailfish-eas / activesync stuff | 09:00 |
chriadam | rawT and rawJ returns no results either | 09:01 |
chriadam | grepping in the buteo repos now also | 09:05 |
chriadam | no rawEv in those either | 09:05 |
chriadam | so I think it's probably just the n-q-p-c | 09:05 |
dcaliste | So if you agree, I can modify the mKCal MR and when you will merge it, the nemo binding one should go in at the same time to be consistent. | 09:05 |
chriadam | I agree - thank you | 09:06 |
chriadam | did MartinS get back to you about the PR he was supposed to look at? | 09:08 |
dcaliste | No, I guess he didn't have time for it yet. | 09:09 |
dcaliste | Also based on previous work on QML bindings for Buteo, I've added a list model for sync results and have a demo application showing the list of passed sync logs. You may have seen the screenshot I put yesterday. | 09:11 |
chriadam | I saw the screenshot, yes, I didn't get a chance to look at the changes. but yes, definitely a very nice thing to enable creation of a proper "sync log" UI | 09:12 |
dcaliste | I still need to implement a filtering method on the model but it looks quite functional to get a page with sync feedback. | 09:12 |
chriadam | I will add a comment to that calendar/timed alarms PR to poke martin again, sec | 09:12 |
dcaliste | For most plugins, the feedback is still sparse with just the time and the status, success or failure. | 09:14 |
dcaliste | Thanks for pushing also the work on timed alarms :) | 09:14 |
chriadam | once the buteo and caldav changes go in, we can look into porting further plugins one by one. in some cases (e.g. google calendar) I think it will be relatively simple, as it already has some similarities to the caldav plugin in terms of per-event statuses. for others it may be trickier, but let's see :-) | 09:16 |
chriadam | I need to poke pvuorela and jpetrell about the sync log UI possibility also - maybe that's something our customers would want also. | 09:17 |
chriadam | b2b / b2g I mean | 09:18 |
chriadam | if it's a priority for them, then we can get more resourcing to port the plugins and do the UI design work also. | 09:18 |
chriadam | let's see :-) | 09:19 |
dcaliste | Yeh great. I could be used also to log the error of email daemon, as related to the recent work of flypig. I can give a look how to export the info from QMF to the email sync plugin. | 09:19 |
chriadam | that would be fantastic | 09:19 |
chriadam | thanks for your work here - the fact that you have built all the pieces needed to test/demo end-to-end (buteo, plugin, qml exposure, demo app) is really great | 09:20 |
chriadam | as it means we can do concrete things rather than talk about what might be. I really appreciate the huge effort you give. | 09:20 |
dcaliste | Thanks. Besides, I will continue to look at your work on transitioning Google sync plugin and give feedback when I can. | 09:20 |
chriadam | thank you :-) | 09:21 |
chriadam | did you have any other topics to discuss tonight? | 09:21 |
dcaliste | No, that's it, thank you for the help making things to go on. Have a good night and a nice week. | 09:21 |
chriadam | thank you - have a great week also | 09:22 |
chriadam | gnight! | 09:22 |
*** Ischwitch is now known as Ingvix | 15:36 | |
*** Ischwitch is now known as Ingvix | 18:11 | |
*** Ischwitch is now known as Ingvix | 18:41 | |
wdehoog | hi. I have a problem with the SDK (3.4 target). the linking phase fails with unresolved references for all used libs. when loggin in on the mersdk vm and running make the linking succeeds. | 21:38 |
Nico[m] | Did your build work before? | 21:39 |
Nico[m] | How did you install your deps? | 21:39 |
ggabriel | wdehoog: perhaps it would be useful to see the actual error, and to see what command exactly you type to make manually in mersdk | 21:42 |
wdehoog | yes it worked before (3.2 target) | 21:43 |
wdehoog | it works if I log in to mersdk vm, execute sb2 -t SailfishOS-3.4.0.24-armv7hl, cd to the build dir and the execute ´make´ | 21:44 |
wdehoog | compiling works in Sailfish IDE so all .o files are there but the linking fails | 21:45 |
ggabriel | weird... you're sure it's the same target? so you don't have a 3.4 RC configured in the IDE? | 21:46 |
Nico[m] | Hm, are you actually running make inside of sb2 with that command? How do your deps get installed? | 21:47 |
wdehoog | I have build the libs on OBS, added a repo in the mersdk vm and installed them (all using sb2) | 21:49 |
Nico[m] | Hm | 21:49 |
Nico[m] | So not installed via rpm depends? | 21:50 |
Nico[m] | Are you sure you reinstalled them in the right target? | 21:50 |
wdehoog | build target SailfishOS-3.4.0.24-armv7hl | 21:50 |
ggabriel | I'd like to see the actual linking error from the IDE | 21:50 |
Nico[m] | I mean, you could have installed the deps into the wrong target, i.e. 3.2 by accident, but yeah, exact error would be useful | 21:51 |
wdehoog | failed https://pastebin.com/N2VXVhwR | 21:54 |
wdehoog | success https://pastebin.com/99fe20Xa | 21:55 |
ggabriel | I'm not sure those are linking errors | 21:58 |
ggabriel | and I don't know how the second time it succeeded | 21:59 |
ggabriel | sorry, can't help tbh :) | 21:59 |
wdehoog | thanks anyway | 21:59 |
wdehoog | other problems I had. rsync and sdk-deploy-rpm were not installed. is that something you have to do yourself? | 22:00 |
Nico[m] | The order of the libraries looks wrong to me :D | 22:00 |
wdehoog | btw I am running on a Oneplus One (community build) | 22:00 |
Nico[m] | <wdehoog "other problems I had. rsync and "> That sounds more like your target wasn't installed correctly | 22:00 |
wdehoog | rsync must be installed on the phone | 22:01 |
Nico[m] | I think I just enabled dev mode | 22:03 |
*** Sellerie7 is now known as Sellerie | 23:33 |
Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!