MySensors icon indicating copy to clipboard operation
MySensors copied to clipboard

Published 20 hours ago • verified publisher icon aisk

Fix fan speed for T2 MacBooks

Open mrousavy opened this issue 4 years ago • 9 comments

On newer MacBooks (T2 chips) the fan info was throwing an error. Also, I've put the temps in the front and sorted by them.

Screenshot 2020-05-12 at 21 48 18

mrousavy avatar May 12 '20 19:05 mrousavy

I also aligned the descriptions/labels left and values on the right. Personal preference which is better, but I think it's easier to read. Also since it's sorted from highest temp to lowest, the most important information is easily readable (because it's first, and on the far right)

Demo: demo

Currently I hardcoded the menu width to 250, don't know if there is a better way to do this.

Maybe apply text styles in a separate loop after creating the labels, and just use the menu.size.width (which now will be the width of the longest text, without spaces) and add some extra width (~20?) to create spacing for the longest text.

See: https://github.com/aisk/MySensors/pull/1/commits/15db3fa7fbdabda6e1f82cf057e2c1981e59106f#diff-95951b3083d0f544bd9afb833c854babR46-R51

mrousavy avatar May 13 '20 15:05 mrousavy

@mrousavy Great job!

But I'm a little busy these days, I'll review it this weekend, sorry!

aisk avatar May 13 '20 16:05 aisk

Hi @mrousavy I like you changes but I think there are some minor changes need to resolve which I have commented. Greet job, many thanks!

aisk avatar May 15 '20 14:05 aisk

Oh, my bad! Didn't notice Xcode wasn't using spaces, should've double checked that.

Also, the README changes shouldn't be in here as well, will remove them right away.

mrousavy avatar May 15 '20 14:05 mrousavy

LGTM, I'll try to test the changes on my machine, wait a minutes.

aisk avatar May 15 '20 14:05 aisk

Looks like the fan speed does not worked my my machine (MacBook Pro Mid 2015, no T2 chips):

image

aisk avatar May 15 '20 15:05 aisk

Hmm that's weird, maybe we should check for device type and then decide between the methods to use fan speed, legacy on non T2 chips and the new one on T2 chips.. unfortunately I don't have a device to test it..

mrousavy avatar May 15 '20 15:05 mrousavy

Good idea, I'll have a try tomorrow.

aisk avatar May 15 '20 15:05 aisk

@mrousavy Hi readly sorry for the late of reply. I just took a look at the original PR from SMCKit, and found they have a do ... catch mechanism, for read the data as it is T2 chip model and then fallback to read data like before.

I copied the change and it works fine. I think in this PR. we can copy the total SCMKit.swift file from the original PR from here: https://github.com/beltex/SMCKit/blob/ff231124ef2c4e7924cefa388bbca8d43c1fcb63/SMCKit/SMC.swift /

aisk avatar Jul 05 '20 13:07 aisk