freakstatic / node-temperature-mqtt

A computer temperature monitor that sends the reads using MQTT
10 stars 2 forks source link

any way to fix CMD popups appearing in PM2 / forever ? #2

Open vikoalucard opened 3 years ago

vikoalucard commented 3 years ago

Hello,

Thank you a lot for this project, it really gives added value where I've found no other tool to collect GPU & CPU temp from remote PCs. I've tested this on windows, and using PM2, each time the interval is triggered, it produces a temporary CMD window, which is quite problematic. I'm no expert in nodeJS and PM2, but I've seen that most of the time, it can be solved by adding an option like windowsHide:true to avoid trigerring the cmd windows

Then it'd be easy to run & autostart on N computers using PM2 / forever (or others like that). I've found the same problem on forever.

freakstatic commented 3 years ago

Hi, thanks for reaching out!

Really interesting to see this small project being used! :smile: Have you try to disable the console.log output by setting to false the "log" property in the config.json file?

vikoalucard commented 3 years ago

@freakstatic thank you for your help. I had already disabled the logs, it didn't help though. btw, the logs only gave the GPU temp, which is what I need most anyway, but it may also not work with my CPU (i9-9900k)

Also, I haven't registered the mqtt devices manually in home assistant. I hoped MQTT could have used "discovery" to create them automatically.

freakstatic commented 3 years ago

I will test this on Windows and let you know once I have fixed. Are you running Windows 10?

I need to investigate a bit about the MQTT Discovery seems like a good feature to have!

vikoalucard commented 3 years ago

Yes, win 10 pro, here are the details in case you have trouble reproducing the issue Edition Windows 10 Pro Version 20H2 Installed on ‎2020-‎08-‎12 OS build 19042.844 Experience Windows Feature Experience Pack 120.2212.551.0

about MQTT discovery: it may require the "homeassistant" prefix to be auto created https://www.home-assistant.io/docs/mqtt/discovery/

freakstatic commented 3 years ago

@vikoalucard I have updated some dependencies, can you try now to see if it collects your CPU temperatures?

freakstatic commented 3 years ago

I also tried the windowsHide: true option that you mention, let me know if it worked :)

vikoalucard commented 3 years ago

Hi @freakstatic , thank you. windowsHide: true has worked the script still only gets GPU temp: only "Graphic card 0: 49" gets displayed

btw, what's the "clientId": "" for in the config.json ? Will you also add the autodiscovery, or should I add the config into Home Assistant ? I could keep using it for GPU temp now that it doesn't show popups 👍

freakstatic commented 3 years ago

Strange seems like the package that I'm using doesn't support your CPU then, maybe I should look for alternatives... The "clientId" it's used to uniquely identify the client when connecting to the MQTT broker, if it's empty it will use a random one.

Tomorrow I will try to add the autodiscovery :)

freakstatic commented 3 years ago

Try to run this in the command line and show me the output pls wmic /namespace:\\root\wmi PATH MSAcpi_ThermalZoneTemperature get CriticalTripPoint,CurrentTemperature /value

vikoalucard commented 3 years ago

OK thank you ! Here it is: PS C:\WINDOWS\system32> wmic /namespace:\root\wmi PATH MSAcpi_ThermalZoneTemperature get CriticalTripPoint,CurrentTemperature /value

CriticalTripPoint=3922 CurrentTemperature=3010

freakstatic commented 3 years ago

wmic - which is used to determine temperature and battery sometimes needs to be run with admin privileges. So if you do not get any values, try to run it again with according privileges. If you still do not get any values, your system might not support this feature. In some cases we also discovered that wmic returned incorrect temperature values.

Are you running Node.js with admin privileges?

vikoalucard commented 3 years ago

Hi, yes I was, it gave the current temp & "CriticalTripPoint" (see my previous comment) I don't really understand the values though... they're not C° degrees without admin priviledge I just get an error

vikoalucard commented 3 years ago

PS. sorry I thought you meant the WMIC command, I've tried running the "node index.js" via an admin powershell, I did get one additional line, but it doesn't repeat, only the GPU is displayed after that (perhaps only the GPU temp is displayed in the console ?)

PS D:_code\automations\node-temperature-mqtt> node .\index.js Connected to broker on mqtt://192.168.75.74:1883 CPU Thread 0: 27.8 Graphic card 0: 50 Graphic card 0: 50 Graphic card 0: 50 Graphic card 0: 50 Graphic card 0: 50 Graphic card 0: 50 Graphic card 0: 50 Graphic card 0: 50 Graphic card 0: 50

freakstatic commented 3 years ago

Damn something really strange is going on there... First it only outputs one of the cores and it stops (probably because the value didn't change). It should only output when the value of that graphic or processor core has changed

freakstatic commented 3 years ago

Can you try to run this script with Node.js?


const si = require('systeminformation');

si.cpu()
  .then(data => {
    console.log('CPU Information:');
    console.log('- manufucturer: ' + data.manufacturer);
    console.log('- brand: ' + data.brand);
    console.log('- speed: ' + data.speed);
    console.log('- cores: ' + data.cores);
    console.log('- physical cores: ' + data.physicalCores);
    console.log('...');
  })
  .catch(error => console.error(error));```
vikoalucard commented 3 years ago

Yes of course, I can try that. But I'm no Node.js expert, in which existing file should I add this? Or how do I run this script alone ?

freakstatic commented 3 years ago

you can just create a file like test.js inside this project folder and run like node .\test.js

vikoalucard commented 3 years ago

OK, here goes: PS D:_code\automations\node-temperature-mqtt> node .\test.js CPU Information:

freakstatic commented 3 years ago

And now:

const si = require('systeminformation');  

si.cpuTemperature().then(data => {
    console.log(data);
})
vikoalucard commented 3 years ago

I got an error at first: TypeError: si.getProcessorTemps is not a function

I replaced the code with: si.cpuTemperature().then(data => { console.log(data); })

then I got: PS D:_code\automations\node-temperature-mqtt> node .\test.js { main: 27.8, cores: [ 27.8 ], max: 27.8, socket: [], chipset: null }

freakstatic commented 3 years ago

yeah sorry I wrote it wrong :)

interesting so systeminformation is only getting the temperature of one core

vikoalucard commented 3 years ago

not that bad, on a single CPU with 16 cores, at least I get a good idea of the overall CPU temp. So it works well in admin mode ? with the latest execution I did get one value for CPU, it may simply not have changed during the test, I can run a game to see the values

vikoalucard commented 3 years ago

the CPU values are also wrong it seems, and they do not get updated: image

freakstatic commented 3 years ago

yeah that doesn't seem right... At least the graphic card values are right?

freakstatic commented 3 years ago

btw I did a commit now to fix the duplicated graphic card temperature values

vikoalucard commented 3 years ago

Yes the GPU values are OK compared to HWinfo64