D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-09-05 Thread Méven Car 
meven marked 2 inline comments as done.
meven added inline comments.

INLINE COMMENTS

> jgrulich wrote in wirelessdevice.h:143
> Please do, you can push it directly, it's just a function rename.

https://cgit.kde.org/networkmanager-qt.git/commit/?id=af98fdba63e32c38008592774436ae6c9b61cd90

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-09-04 Thread Jan Grulich 
jgrulich added inline comments.

INLINE COMMENTS

> meven wrote in wirelessdevice.h:143
> I chose lastRequestScanTime to have RequestScan in the name, that is the 
> function, it keeps the last time called from.
> 
> The Time suffix does not feel great indeed.
> 
> I can make an adjustment to `lastRequestScan`, I will adjust D23578 
>  then

Please do, you can push it directly, it's just a function rename.

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-09-04 Thread Méven Car 
meven added inline comments.

INLINE COMMENTS

> jgrulich wrote in wirelessdevice.h:143
> Thinking about it now, wouldn't be this wording better?
> 
> QDateTime lastScanRequestTime() const;
> 
> Maybe even without the "Time" at the end so it's consistent with "lastScan".

I chose lastRequestScanTime to have RequestScan in the name, that is the 
function, it keeps the last time called from.

The Time suffix does not feel great indeed.

I can make an adjustment to `lastRequestScan`, I will adjust D23578 
 then

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-09-04 Thread Jan Grulich 
jgrulich added inline comments.

INLINE COMMENTS

> wirelessdevice.h:143
> + */
> +QDateTime lastRequestScanTime() const;
>  /**

Thinking about it now, wouldn't be this wording better?

QDateTime lastScanRequestTime() const;

Maybe even without the "Time" at the end so it's consistent with "lastScan".

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-09-02 Thread Méven Car 
meven added a comment.


  D23578  is the next step, but it is not 
in great shape.
  To do things properly, it will require quite some changes I fear.

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-09-02 Thread Méven Car 
This revision was automatically updated to reflect the committed changes.
Closed by commit R282:d9cb570cbef4: Add property lastScanTime and 
lastRequestTime to WirelessDevice (authored by meven).

REPOSITORY
  R282 NetworkManagerQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23576?vs=65208=65210

REVISION DETAIL
  https://phabricator.kde.org/D23576

AFFECTED FILES
  src/utils.cpp
  src/utils.h
  src/wirelessdevice.cpp
  src/wirelessdevice.h
  src/wirelessdevice_p.h

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-09-02 Thread Méven Car 
meven edited the summary of this revision.

REPOSITORY
  R282 NetworkManagerQt

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-09-02 Thread Jan Grulich 
jgrulich accepted this revision.

REPOSITORY
  R282 NetworkManagerQt

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-09-02 Thread Méven Car 
meven updated this revision to Diff 65208.
meven added a comment.


  Add a ref to the returned date

REPOSITORY
  R282 NetworkManagerQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23576?vs=65204=65208

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D23576

AFFECTED FILES
  src/utils.cpp
  src/utils.h
  src/wirelessdevice.cpp
  src/wirelessdevice.h
  src/wirelessdevice_p.h

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-09-02 Thread Jan Grulich 
jgrulich accepted this revision.
jgrulich added a comment.
This revision is now accepted and ready to land.


  Sorry, I missed that last one. Once it's fixed it's ready to go.

INLINE COMMENTS

> wirelessdevice.h:232
> + */
> +void lastScanChanged(const QDateTime timestamp);
>  

const QDateTime 

REPOSITORY
  R282 NetworkManagerQt

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-09-02 Thread Méven Car 
meven updated this revision to Diff 65204.
meven marked an inline comment as done.
meven added a comment.


  Review, remove incorrect doc

REPOSITORY
  R282 NetworkManagerQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23576?vs=65002=65204

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D23576

AFFECTED FILES
  src/utils.cpp
  src/utils.h
  src/wirelessdevice.cpp
  src/wirelessdevice.h
  src/wirelessdevice_p.h

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-09-02 Thread Jan Grulich 
jgrulich added inline comments.

INLINE COMMENTS

> utils.h:86
> +
> +QDateTime clockBootTimeToQDateTime(const qlonglong );
>  }

1. Missing NETWORKMANAGERQT_EXPORT
2. I would remove 'Q' from the name → clockBootTimeToDateTime
3. just qlonglong clockBoottime, no const and &

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-08-30 Thread Méven Car 
meven marked 4 inline comments as done.
meven added a comment.


  I have implemented the CLOCK_BOOTTIME conversion to QDateTime in `QDateTime 
NetworkManager::clockBootTimeToQDateTime`

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-08-30 Thread Méven Car 
meven updated this revision to Diff 65002.
meven added a comment.


  Convert the LastScan value to a QDateTime, update function and doc in 
consequence

REPOSITORY
  R282 NetworkManagerQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23576?vs=64987=65002

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D23576

AFFECTED FILES
  src/utils.cpp
  src/utils.h
  src/wirelessdevice.cpp
  src/wirelessdevice.h
  src/wirelessdevice_p.h

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-08-30 Thread Méven Car 
meven added inline comments.

INLINE COMMENTS

> jgrulich wrote in wirelessdevice.cpp:250
> Then cannot you work with it as with qlonglong? It shouldn't matter then if 
> it's in CLOCK_BOOTIME .

I need to be able to know the elapsed time since LastScan. So it doesn't matter 
as long as I manage to get a CLOCK_BOOTIME value for current time.

Anyway I am thinking about adding a short util function somewhere to convert 
CLOCK_BOOTTIME to QDateTime.

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-08-30 Thread Jan Grulich 
jgrulich added inline comments.

INLINE COMMENTS

> meven wrote in wirelessdevice.cpp:250
> LastScan is in CLOCK_BOOTIME which is complicated to work with.
> So I use naively the signal time rather than the LastScan value.
> This will need a bit of work just to work with it:
> https://github.com/NetworkManager/NetworkManager/blob/a7d8fe0ea5eb7be42a86527226ea54fd221fb1b4/shared/nm-glib-aux/nm-time-utils.c#L193

Then cannot you work with it as with qlonglong? It shouldn't matter then if 
it's in CLOCK_BOOTIME .

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-08-30 Thread Méven Car 
meven marked 2 inline comments as done.

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-08-30 Thread Méven Car 
meven updated this revision to Diff 64987.
meven added a comment.


  Mention nm version limitation

REPOSITORY
  R282 NetworkManagerQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23576?vs=64983=64987

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D23576

AFFECTED FILES
  src/wirelessdevice.cpp
  src/wirelessdevice.h
  src/wirelessdevice_p.h

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-08-30 Thread Méven Car 
meven added a comment.


  In D23576#522341 , @jgrulich wrote:
  
  > In D23576#522340 , @meven wrote:
  >
  > > I am hesitant to add a second field "previousScan" that would store the 
previous lastScan timeStamp when a new one arrives.
  > >  This would be used to compute the time elapsed between the scans which 
is necessary to do since Network manager requires scans to be at least 10 
seconds apart :
  > >  
https://github.com/NetworkManager/NetworkManager/blob/master/src/devices/wifi/nm-device-wifi.c#L1205
  >
  >
  > That would be useful I guess and could be used in plasma-nm.
  >
  > 1. You should set initial lastScan property to "-1" which should indicate 
there was no scan attempt
  > 2. Mention that it will work only when NM 1.12+ is running, older NM 
releases don't support this property
  > 3. Above mentioned will be a problem when you would like to rely on that in 
plasma-nm, because we should be working even with older NM versions
  
  
  
  
  1. I missed your comment and made important code change, instead of using the 
LastScan property, I use QDateTime to retain requestScan event and LastScan 
changes event.
  
  2. will do
  
  3. My code in plasma should be compatible already D23578 


INLINE COMMENTS

> jgrulich wrote in wirelessdevice.cpp:250
> Don't use QDateTime::currentDateTime() as it might be different from the 
> actual value returned by NetworkManager.

LastScan is in CLOCK_BOOTIME which is complicated to work with.
So I use naively the signal time rather than the LastScan value.
This will need a bit of work just to work with it:
https://github.com/NetworkManager/NetworkManager/blob/a7d8fe0ea5eb7be42a86527226ea54fd221fb1b4/shared/nm-glib-aux/nm-time-utils.c#L193

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-08-30 Thread Jan Grulich 
jgrulich added inline comments.

INLINE COMMENTS

> wirelessdevice.h:133
> + * The time when the last "LastScan" property change was received
> + * @since 5.62.0
> + * @return

Add something like:
@note will always return invalid QDateTime when runtime NM < 1.12

> wirelessdevice.h:228
> + * @since 5.62.0
> + * @see lastScanTime
> + */

Add something like:
@note will never be emitted when runtime NM < 1.12

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-08-30 Thread Jan Grulich 
jgrulich added inline comments.

INLINE COMMENTS

> wirelessdevice.cpp:250
> +} else if (property == QLatin1String("LastScan")) {
> +lastScanTime = QDateTime::currentDateTime();
> +Q_EMIT q->lastScanTimeChanged(lastScanTime);

Don't use QDateTime::currentDateTime() as it might be different from the actual 
value returned by NetworkManager.

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-08-30 Thread Méven Car 
meven added a dependent revision: D23578: Before requesting a scan, check the 
time threshold.

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-08-30 Thread Méven Car 
meven retitled this revision from "Add property lastScan to WirelessDevice and 
associated change signal" to "Add property lastScanTime and lastRequestTime to 
WirelessDevice".
meven edited the summary of this revision.

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

2019-08-30 Thread Méven Car 
meven edited the summary of this revision.

REPOSITORY
  R282 NetworkManagerQt

REVISION DETAIL
  https://phabricator.kde.org/D23576

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns