Closed wimleers closed 1 year ago
:white_check_mark: Pre-checks completed successfully.
A basic config of
{
"platform": "SMAHomeManager",
"_bridge": {
"username": "0E:E7:D4:44:A2:0A",
"port": 46564
}
}
sends Homebridge into a crash loop:
[26/03/2023, 18:04:39] Registering platform 'homebridge-sma-home-manager.SMAHomeManager'
[26/03/2023, 18:04:39] [homebridge-sma-home-manager] Loaded homebridge-sma-home-manager v1.1.1 child bridge successfully
[26/03/2023, 18:04:39] Loaded 0 cached accessories from cachedAccessories.0EE7D444A20A.
/usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:285
Object.keys(signalNames).forEach(id => {
^
TypeError: Cannot read properties of undefined (reading 'offGrid')
at /usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:286:35
at Array.forEach (<anonymous>)
at SMAHomeManager.accessories (/usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:285:28)
at /usr/local/lib/node_modules/homebridge/src/bridgeService.ts:534:24
at new Promise (<anonymous>)
at BridgeService.loadPlatformAccessories (/usr/local/lib/node_modules/homebridge/src/bridgeService.ts:528:12)
at ChildBridgeFork.startBridge (/usr/local/lib/node_modules/homebridge/src/childBridgeFork.ts:155:36)
[26/03/2023, 18:04:39] [homebridge-sma-home-manager] Child bridge process ended
[26/03/2023, 18:04:39] [homebridge-sma-home-manager] Process Ended. Code: 1, Signal: null
[26/03/2023, 18:04:46] [homebridge-sma-home-manager] Restarting Process...
Also does your plugin ever wait for the didFinishLaunching
event? Im not sure if your plugin type needs to or not. But I wonder if this is the reason of the logging
[26/03/2023, 18:23:24] [homebridge-sma-home-manager] This plugin is taking long time to load and preventing Homebridge from starting. See https://homebridge.io/w/JtMGR for more info.
and the icon never turning to green?
@bwp91
homebridge-config-ui-x
would never result in that sort of configuration as far as I can tell? π
(Over here it typically takes a few seconds to discover the devices, connect, and then launch.)
Hi @wimleers
Re (1) - there are still plenty of users who configure stuff directly with JSON, so we just need to make sure that any 'config issues' are caught properly by its plugin - to make sure that it does not send homebridge into a crash loop and therefore not stop any other plugin from functioning. homebridge-config-ui-x
has made things so much easier for a lot of users, but please don't assume that every user will be using this.
Re (2) - okay π - no action needed from you, thanks for explaining
Makes sense, will address that!
Great! Once ready, please reply on here with a comment just saying /check
and we can try again π
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
I will follow up on this in the coming week.
I can't seem to find documentation on what the recommended pattern is for when the configuration is not valid, in either of these places:
So I went with throw new Error
with a helpful message. See https://github.com/wimleers/homebridge-sma-home-manager/commit/1b00ef68eee2e94afbd691ebeabd5961016f18ba.
Released in https://github.com/wimleers/homebridge-sma-home-manager/releases/tag/1.1.4.
/check
:white_check_mark: Pre-checks completed successfully.
Hi @wimleers thanks for trying to sort this out.
The issue remains that you are throwing an error here. In my case I just installed your plugin via the homebridge-config-ui-x, and when the config screen appeared I just clicked save straight away (and then enabled child bridge). This results in a config of:
{
"signals": {
"offGrid": true,
"noSun": true,
"highImport": false
},
"platform": "SMAHomeManager",
"_bridge": {
"username": "0E:16:95:1D:EA:89",
"port": 40942
}
}
This is sending homebridge into a crash loop with this error:
[12/06/2023, 23:25:50] [homebridge-sma-home-manager] Restarting Process...
[12/06/2023, 23:25:52] [homebridge-sma-home-manager] Launched child bridge with PID 9802
[12/06/2023, 23:25:52] Registering platform 'homebridge-sma-home-manager.SMAHomeManager'
[12/06/2023, 23:25:52] [homebridge-sma-home-manager] Loaded homebridge-sma-home-manager v1.1.4 child bridge successfully
/usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:45
['signals', 'surplusSignals'].forEach(requiredTopLevelKey => {
^
Error: Invalid configuration: surplusSignals key is missing. See config.schema.json.
at /usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:47:10
at Array.forEach (<anonymous>)
at new SMAHomeManager (/usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:45:32)
at ChildBridgeFork.startBridge (/usr/local/lib/node_modules/homebridge/src/childBridgeFork.ts:150:42)
[12/06/2023, 23:25:52] [homebridge-sma-home-manager] Child bridge process ended
[12/06/2023, 23:25:52] [homebridge-sma-home-manager] Process Ended. Code: 1, Signal: null
[12/06/2023, 23:25:59] [homebridge-sma-home-manager] Restarting Process...
I appreciate there are not any best practice guides with what to do in the case that a user has an invalid config.
What I personally do with my plugins is just to log a warning that the config is invalid and stop the plugin from loading any more.
For example in your code here:
I would simply use a return statement
log.warning('invalid config etc');
return;
so that the user (assuming they look at the logs) can see that there is an issue with the config, but by not throwing an error, this does not send homebridge into a crash loop and so does not affect other plugins.
Will do. Thanks for taking the time to elaborate!
/check
The following pre-checks failed:
:x: Failed to test plugin. :x: Command failed: docker build -t check .
Comment /check
to run checks again.
Ah, this time there's no bot involved? π π Sorry about that!
EDIT: looks like GitHub was just malfunctioning π€· None of the github-actions
stuff appeared when I wrote this!
/check
The following pre-checks failed:
:x: Failed to test plugin. :x: Command failed: docker build -t check .
Comment /check
to run checks again.
/check
:white_check_mark: Pre-checks completed successfully.
You probably meant log.warn
in your code instead of log.warning
π
Still getting a (new logging) crash loop:
[16/06/2023, 20:33:59] [homebridge-sma-home-manager] Loaded homebridge-sma-home-manager v1.1.5 child bridge successfully
/usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:45
['signals', 'surplusSignals'].forEach(requiredTopLevelKey => {
^
TypeError: log.warning is not a function
at /usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:47:8
at Array.forEach (<anonymous>)
at new SMAHomeManager (/usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:45:32)
at ChildBridgeFork.startBridge (/usr/local/lib/node_modules/homebridge/src/childBridgeFork.ts:150:42)
LOL! I didn't test it this time, I just literally copy/pasted what you wrote at https://github.com/homebridge/verified/issues/527#issuecomment-1588197545 π
Will fix.
Ah sorry! π
/check
:white_check_mark: Pre-checks completed successfully.
Do you have some time to message-chat on the homebridge discord?
If you're already on there, please send me a dm, my username is bwp91
!
Iβm not on Discord π Does email also work?
Email is good it's bwp91@icloud.com
Hi @wimleers
I think the issue here is the return
statement isn't doing anything since it is in the callback of a foreach
['signals', 'surplusSignals'].forEach(requiredTopLevelKey => {
if (!Object.keys(config).includes(requiredTopLevelKey)) {
log.warn(`Invalid configuration: ${requiredTopLevelKey} key is missing. See config.schema.json.`);
return;
}
});
basically, even if there is an invalid config, it is logging, returning inside this function, but then still continuing with the rest of the plugin initialisation.
Since there are only two 'keys' here, it may be easier just to
if (!Object.keys(config).includes('signals')) {
log.warn(`Invalid configuration: signals key is missing. See config.schema.json.`);
return;
}
if (!Object.keys(config).includes('surplusSignals')) {
log.warn(`Invalid configuration: surplusSignals key is missing. See config.schema.json.`);
return;
}
or a for
loop, or a try
/catch
.
But at the moment that return;
statement is redundant and even if a config item is missing, the logwarning is processed, but the rest of the plugin init continues
Hi @wimleers any update on this?
Sorry, I was enjoying https://gentsefeesten.stad.gent/en at the time!
Will sort this out in the next few days π
returning inside this function, but then still continuing with the rest of the plugin initialisation.
OMG π€¦ π
Released: https://github.com/wimleers/homebridge-sma-home-manager/releases/tag/1.1.7
/check
:white_check_mark: Pre-checks completed successfully.
Just wanted to pick up on one thing, this is nothing to do with verified or not!
after installing plugin I see this screen, the recommendations
link doesn't work, it opens a new window to
http://homebridge.local/#SMAHomeManager-recommendations
which basically just shows the home page of homebridge ui
With regards to verification, on a new install of this plugin, after clicking Save on screenshot above, setting up as a child bridge and restarting homebridge I am getting this constant restart:
[16/08/2023, 22:23:14] [homebridge-sma-home-manager] Invalid configuration: surplusSignals key is missing. See config.schema.json.
/usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:285
const liveService = new Service.CustomPowerMonitor();
^
TypeError: Service.CustomPowerMonitor is not a constructor
at SMAHomeManager.accessories (/usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:285:23)
at /usr/local/lib/node_modules/homebridge/src/bridgeService.ts:534:24
at new Promise (<anonymous>)
at BridgeService.loadPlatformAccessories (/usr/local/lib/node_modules/homebridge/src/bridgeService.ts:528:12)
at ChildBridgeFork.startBridge (/usr/local/lib/node_modules/homebridge/src/childBridgeFork.ts:155:36)
[16/08/2023, 22:23:14] [homebridge-sma-home-manager] Child bridge process ended
[16/08/2023, 22:23:14] [homebridge-sma-home-manager] Process Ended. Code: 1, Signal: null
[16/08/2023, 22:23:21] [homebridge-sma-home-manager] Restarting Process...
[16/08/2023, 22:23:23] [homebridge-sma-home-manager] Launched child bridge with PID 21925
[16/08/2023, 22:23:23] Registering platform 'homebridge-sma-home-manager.SMAHomeManager'
[16/08/2023, 22:23:23] [homebridge-sma-home-manager] Loaded homebridge-sma-home-manager v1.1.7 child bridge successfully
[16/08/2023, 22:23:23] [homebridge-sma-home-manager] Invalid configuration: surplusSignals key is missing. See config.schema.json.
/usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:285
const liveService = new Service.CustomPowerMonitor();
^
TypeError: Service.CustomPowerMonitor is not a constructor
at SMAHomeManager.accessories (/usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:285:23)
at /usr/local/lib/node_modules/homebridge/src/bridgeService.ts:534:24
at new Promise (<anonymous>)
at BridgeService.loadPlatformAccessories (/usr/local/lib/node_modules/homebridge/src/bridgeService.ts:528:12)
at ChildBridgeFork.startBridge (/usr/local/lib/node_modules/homebridge/src/childBridgeFork.ts:155:36)
[16/08/2023, 22:23:23] [homebridge-sma-home-manager] Child bridge process ended
[16/08/2023, 22:23:23] [homebridge-sma-home-manager] Process Ended. Code: 1, Signal: null
[16/08/2023, 22:23:30] [homebridge-sma-home-manager] Restarting Process...
[16/08/2023, 22:23:31] [homebridge-sma-home-manager] Launched child bridge with PID 21939
[16/08/2023, 22:23:31] Registering platform 'homebridge-sma-home-manager.SMAHomeManager'
[16/08/2023, 22:23:31] [homebridge-sma-home-manager] Loaded homebridge-sma-home-manager v1.1.7 child bridge successfully
[16/08/2023, 22:23:31] [homebridge-sma-home-manager] Invalid configuration: surplusSignals key is missing. See config.schema.json.
/usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:285
const liveService = new Service.CustomPowerMonitor();
^
TypeError: Service.CustomPowerMonitor is not a constructor
at SMAHomeManager.accessories (/usr/local/lib/node_modules/homebridge-sma-home-manager/index.js:285:23)
at /usr/local/lib/node_modules/homebridge/src/bridgeService.ts:534:24
at new Promise (<anonymous>)
at BridgeService.loadPlatformAccessories (/usr/local/lib/node_modules/homebridge/src/bridgeService.ts:528:12)
at ChildBridgeFork.startBridge (/usr/local/lib/node_modules/homebridge/src/childBridgeFork.ts:155:36)
[16/08/2023, 22:23:31] [homebridge-sma-home-manager] Child bridge process ended
[16/08/2023, 22:23:31] [homebridge-sma-home-manager] Process Ended. Code: 1, Signal: null
So itβs still not working when not configured correctly.
Iβll do a separate Homebridge install just to test that edge case. But Iβm kinda baffled because I did exactly what you asked π π
Sorry Iβm taking up so much of your time. Definitely the opposite of my intent, because I too am an open source software maintainer, although in a very different software world.
@wimleers no problem at all sometimes unblocking one issue just leads to another, very used to this and all good ππ»
@wimleers i was just having a look at the line of code that was causing the issue and im afraid when it comes to custom services and characteristics I am a bit clueless π
Hah! I get that. It's a bit of a maze, but they are so very powerful! π It works great in Eve, Home+, Controller, etc. Really unfortunate that the official Home app is so incredibly limited :(
custom characteristics are common with (verified) plugins, I personally have implemented these in my plugins that are verified. So it's not a show-stopper, in your case it just seems to be an issue with implementation.
Have you tried wiping your accessory cache and starting from new?
Custom characteristics don't normally, and should not, affect a standard homebridge user who only has the home app
This weekend I'll get this done π
Custom characteristics don't normally, and should not, affect a standard homebridge user who only has the home app
Agreed! All I meant to say was: the Home app does not allow setting up automations based on custom characteristics.
The custom service/characteristic was a mere distraction. The validation warning caused an early return in the constructor, which caused the custom service to not be defined yet. Because despite the early return, HomeBridge will still call accessories()
.
I first added checks to have that return an empty list of accessories if the config was invalid.
But β¦ there's a much, much simpler solution I should've adopted months ago: adding fallback logic to just allow no config whatsoever. I can't believe I didn't think of this months ago. 99% of the logic to deal with that was already in place! π
So the fix now is A) deleting the silly validation logic, B) adding || []
βΒ really, that's it π
There you go: https://github.com/wimleers/homebridge-sma-home-manager/releases/tag/1.1.8
/check
:white_check_mark: Pre-checks completed successfully.
deleting the silly validation logic im sure this was my 'silly validation logic' right?!
One question I have is why the long plugin name like homebridge-sma-home-manager
, and why not something shorter like homebridge-sma
?
I'm not familiar with the sma brand, but does your plugin generally support all their devices?
but does your plugin generally support all their devices?
No, it doesn't.
I'd say >90% of SMA users are consumers with a solar (photovoltaic) inverter of theirs: https://www.sma.de/en/products/solar-inverters
The remaining 10% is probably divided between:
For all of the above, the SMA Home Manager (https://www.sma.de/en/products/monitoring-control/sunny-home-manager) is an optional add-on. But only with this do you get correct/complete production & consumption information. And only this broadcasts it in real time (one UDP packet per second, broadcast via IP multicast): Watt produced and consumed.
That's why I specifically named it "SMA Home Manager", because without it, this plugin is useless. It does also specifically talk to the solar inverter, but only for: live A
and V
(and cumulative production today, whether there is a fault, etc). But β¦ this happens via ModBus, which does not support reading data once per second π Even once per 10 seconds will cause problems!
The original repo (https://github.com/codyc1515/homebridge-sma-inverter) from which I forked was barely maintained and very buggy, but did not require the SMA Home Manager: it supported only solar inverters.
So:
(I bet that was more information you were hoping for? π )
A yes or no would have been sufficient for me (!) but actually your extended response will be useful for a paper trail on here, so thank you for the detail, it's good for reference. Especially if there is ever a future sma
plugin that wants to verify.
I am just personally on a bit of a mission at the moment to try and keep plugins with simple names where appropriate.
Link To GitHub Repo
https://github.com/wimleers/homebridge-sma-home-manager
Link To NPM Package
https://www.npmjs.com/package/homebridge-sma-home-manager