victronenergy / venus

Victron Energy Unix/Linux OS
https://github.com/victronenergy/venus/wiki
593 stars 75 forks source link

Update SignalK to latest #1099

Closed mpvader closed 1 year ago

mpvader commented 1 year ago

Please check signalk-server itself, wanted version is v1.46.3.

But also check each of the pre-installed plugins.

dirkjanfaber commented 1 year ago

Put the new recipes here: https://github.com/victronenergy/meta-victronenergy/commit/ae03223846944dfb1239780c32030d7e6bb0b1d4

This is branched from mva/nodejs, so I tested it with nodejs 18. Which was also used building the npm shrinkwrap file (so that file changed more than usual).

mpvader commented 1 year ago

Ok thanks. Why the set HOME commit?

dirkjanfaber commented 1 year ago

The HOME variable is used by Signalk to find its configuration files. On line 13 of /usr/lib/node_modules//signalk-server/node_modules/application-config-path/index.js it does a:

  return path.join(process.env.HOME, '.config', name)

When undefined, it won't start and throws an error. This is the best fix I could think of.

mpvader commented 1 year ago

Ok. Strange, since until now this wasn’t necessary and the thing did work. I’m trying to see if there is extra testing needed or something similar.

mpvader commented 1 year ago

@sbender9 / @tkurki, any thoughts on why we now need to set a home variable?

Perhaps its because we moved from NodeJS v14 to NodeJS v18? The other way to look at it is: any harm in setting that variable?

tkurki commented 1 year ago

Oh, dependency on HOME is a side effect of https://github.com/SignalK/signalk-server/commit/d91d1fd23305aac64f55209ebf4343852a6aefde. This is by accident, not by design, but I think something we can work with. We could probably tweak the code to proceed without failing without HOME.

SSL support out of the box in SK Server was meant to promote https usage, but I think there are very few people using this feature. Proper https requires certificates, which is beyond Joe Boater anyway, so having this work "always" is not a priority.

mpvader commented 1 year ago

Hi @tkurki , thanks for the quick reply.

I don't have any issue with the HOME env variable being required; I just wanted to understand where its (suddenly) coming from.

I know enough - thanks!