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

Info page #32

Merged
merged 24 commits into from Sep 20, 2021
Merged

Info page #32

merged 24 commits into from Sep 20, 2021

Conversation

Jupi007
Copy link
Collaborator

@Jupi007 Jupi007 commented Aug 3, 2021

Work in progress branch for the info page.

Capture d’écran du 2021-09-19 00-23-54

Closes #24

@Jupi007 Jupi007 marked this pull request as draft August 3, 2021 17:29
@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 12, 2021

Get graphic infos (can't found a library sadly) ;

https://github.com/LolzDEV/linux_system_info/pull/3 😉

@Jupi007
Copy link
Collaborator Author

Jupi007 commented Sep 13, 2021

Get graphic infos (can't found a library sadly) ;

LolzDEV/linux_system_info#3 wink

Awesome!
Big thanks to you 😃

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 14, 2021

Get 32/64 bit info ;

One option would be to use dart:ffi to extract the size of a pointer:

import 'dart:ffi';

bool get is32Bit => sizeOf<IntPtr>() == 4;
bool get is64Bit => sizeOf<IntPtr>() == 8;

Given that Flutter doesn't officially support 32-bit Linux, however, it's quite unlikely that anyone will ever run the Settings app on a 32-bit system. Technically, it's possible to build Flutter for 32-bit Linux (e.g. flutter-pi) if you're willing to go deep enough, though. 😄

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 14, 2021

Get computer name.

The dbus package has an example that is a good starting point:

import 'package:dbus/dbus.dart';

var client = DBusClient.system();
var object = DBusRemoteObject(client, destination: 'org.freedesktop.hostname1', path: DBusObjectPath('/org/freedesktop/hostname1'));
var hostname = await object.getProperty('org.freedesktop.hostname1', 'Hostname');
print('hostname: ${hostname.toNative()}');
await client.close();

What gnome-control-center does:

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 14, 2021

Get wayland/X.org info ;

import 'dart:io';

bool get isX11 => sessionType == 'x11';
bool get isWayland => sessionType == 'wayland';
String? get sessionType => Platform.environment['XDG_SESSION_TYPE'];

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 14, 2021

Get gnome-shell infos ;

https://github.com/LolzDEV/linux_system_info/pull/4

@Jupi007
Copy link
Collaborator Author

Jupi007 commented Sep 17, 2021

Here is the current state of the info page:

Capture d’écran du 2021-09-17 20-52-29

@jpnurmi I integrated as I could all your advice (surly like a big noob 😓). Big thanks for your help :)

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 17, 2021

Hi @Jupi007, have you looked at the series of view model PRs from @mivoligo? It would be great to use the pattern here too so we could move the D-Bus stuff etc. out of the widget tree. :)

Disk infos with unix_disk_space

Let's try to figure out something nicer. That package looks pretty inactive and it's creating a child process to run the df command. 👀 I think we could try to use udisks.dart instead. 🙂

Make hostname editable.

I was playing with this. I can push a service class to help with setting the pretty hostname and sanitizing the static hostname.

@Jupi007 Jupi007 marked this pull request as ready for review September 17, 2021 20:29
@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 17, 2021

import 'package:filesize/filesize.dart';
import 'package:udisks/udisks.dart';

Future<void> main() async {
  final client = UDisksClient();
  await client.connect();
  final total = client.drives.fold<int>(0, (sum, drive) => sum + drive.size);
  print(filesize(total)); // 954 GB
  await client.close();
}

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 18, 2021

Very nice! 👍 Are the only remaining things GPU info and disk capacity?

@Jupi007
Copy link
Collaborator Author

Jupi007 commented Sep 18, 2021

Very nice! +1 Are the only remaining things GPU info and disk capacity?

Yes 😃
I also plan to add a popup window for editing the hostname.

@Feichtmeier
Copy link
Member

Feichtmeier commented Sep 18, 2021

Very nice! +1 Are the only remaining things GPU info and disk capacity?

Yes 😃
I also plan to add a popup window for editing the hostname.

Good idea, then we could fix this issue #59 (a half of the issue) within this PR. I think another page for only the hostname is pretty pointless

@Jupi007
Copy link
Collaborator Author

Jupi007 commented Sep 18, 2021

Capture d’écran du 2021-09-19 00-23-54
Capture d’écran du 2021-09-19 00-24-01

Tada ✨

@Jupi007
Copy link
Collaborator Author

Jupi007 commented Sep 19, 2021

P.S. I think GCC got the terminology and spelling right. Any reason to deviate? ("64-bit" vs. "64 Bits", "GNOME" vs. "Gnome", "Windowing System" vs. "Window server")

About the terminology, I based my work on the french version where it is written "64 Bits" 😅

Are those the only wrong terms?

@Jupi007
Copy link
Collaborator Author

Jupi007 commented Sep 19, 2021

Looks very nice!

I get this error - am I doing something wrong?

[ERROR:flutter/lib/ui/ui_dart_state.cc(209)] Unhandled Exception: Bad state: Cannot add event after closing
#0      _StreamController.add (dart:async/stream_controller.dart:553:24)
#1      HostnameService._updateHostname
#2      HostnameService._initProperties
<asynchronous suspension>
#3      HostnameService.init
<asynchronous suspension>
#4      InfoModel.init
<asynchronous suspension>

I got it too. It looks related to the HostnameService. @jpnurmi What do you think of?

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 19, 2021

@Feichtmeier There's something wrong with the master-detail navigation. Whenever you navigate to a page, it's creating 3 instances of that page, and immediately disposes one of them (before the model & service had a chance to even finish initialization).

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 19, 2021

@Feichtmeier Did you ever try https://github.com/jpnurmi/flutter_master_detail_example? I don't know if it's perfect either, but it's not trying to do anything "smart" tricks with custom overlay routes at least.

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 19, 2021

P.S. We could easily guard HostnameService against this, but it wouldn't fix the problem because then the model will be next in line and explode in its initialization routines... And any other view+model with asynchronous initialization will face the same issue. :(

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 19, 2021

Here's an example how we can guard against "too early" disposal:

diff --git a/lib/view/pages/info/hostname_service.dart b/lib/view/pages/info/hostname_service.dart
index 4ccb402..1295499 100644
--- a/lib/view/pages/info/hostname_service.dart
+++ b/lib/view/pages/info/hostname_service.dart
@@ -62,8 +62,12 @@ class HostnameService {
     _prettyHostname = prettyHostname ?? _prettyHostname;
     _staticHostname = staticHostname ?? _staticHostname;
 
-    _hostnameController.add(this.hostname);
-    _staticHostnameController.add(this.staticHostname);
+    if (!_hostnameController.isClosed) {
+      _hostnameController.add(this.hostname);
+    }
+    if (!_staticHostnameController.isClosed) {
+      _staticHostnameController.add(this.staticHostname);
+    }
   }
 
   Future<void> _initProperties() async {
diff --git a/lib/view/pages/info/info_model.dart b/lib/view/pages/info/info_model.dart
index 1707e01..0f6e90b 100644
--- a/lib/view/pages/info/info_model.dart
+++ b/lib/view/pages/info/info_model.dart
@@ -20,6 +20,7 @@ class InfoModel extends ChangeNotifier {
 
   String _gpuName = '';
   int? _diskCapacity;
+  bool _wasDisposed = false;
 
   Future<void> init() async {
     await _hostnameService.init();
@@ -32,7 +33,9 @@ class InfoModel extends ChangeNotifier {
       _diskCapacity = _uDisksClient.drives.fold<int>(0, (sum, drive) => sum + drive.size);
     });
 
-    notifyListeners();
+    if (!_wasDisposed) {
+      notifyListeners();
+    }
   }
 
   String get hostname => _hostnameService.hostname;
@@ -57,6 +60,7 @@ class InfoModel extends ChangeNotifier {
 
   @override
   void dispose() {
+    _wasDisposed = true;
     _hostnameService.dispose();
     _uDisksClient.close();
     super.dispose();

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 19, 2021

Add safe_change_notifier as a dependency, and change ChangeNotifier to SafeChangeNotifier. :)

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 19, 2021

It seems to be a surprisingly common problem, so I made quickly a package out of it. :P I'll write docs and tests later.

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 19, 2021

@Jupi007 Are you not using automatic formatting for Dart code? I get a bit of diff here when touching files.

@Jupi007
Copy link
Collaborator Author

Jupi007 commented Sep 19, 2021

Add safe_change_notifier as a dependency, and change ChangeNotifier to SafeChangeNotifier. :)

Done :)

@Jupi007 Are you not using automatic formatting for Dart code? I get a bit of diff here when touching files.

I don't think I have automatic formatting enabled. Does this require a specific ext?

@mivoligo
Copy link
Collaborator

I don't think I have automatic formatting enabled. Does this require a specific ext?

If you use VSCode, look in Settings for "sdk formatter" and make sure it's enabled
Selection_003

@Jupi007
Copy link
Collaborator Author

Jupi007 commented Sep 19, 2021

Just checked, and it is enabled. Should I do anything special to run the formatter?
I enabled formatOnSave and now it works.

@Feichtmeier
Copy link
Member

Just checked, and it is enabled. Should I do anything special to run the formatter?

CTRL + SHIFT + I formats the document just in case the auto format does not work

Copy link
Contributor

@jpnurmi jpnurmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

pubspec.yaml Outdated
@@ -6,7 +6,7 @@ publish_to: 'none' # Remove this line if you wish to publish to pub.dev
version: 1.0.0+1

environment:
sdk: ">=2.12.0 <3.0.0"
sdk: ">=2.14.0 <3.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpnurmi jpnurmi changed the title Wip info page Info page Sep 20, 2021
@Feichtmeier
Copy link
Member

Shall we merge?

@Jupi007
Copy link
Collaborator Author

Jupi007 commented Sep 20, 2021

Ok for me.

@jpnurmi ?

@jpnurmi
Copy link
Contributor

jpnurmi commented Sep 20, 2021

Go for it! 👍

@Jupi007 Jupi007 merged commit dc10a11 into master Sep 20, 2021
@Jupi007 Jupi007 deleted the info_page branch September 20, 2021 08:15
@Jupi007
Copy link
Collaborator Author

Jupi007 commented Sep 20, 2021

Thanks a lot for your help on this @jpnurmi :)
I couldn't have done it without you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing page: Info
4 participants