fix broken solarwatt myreserve template

[t3r]

• fix broken "pv" computation by computing VPV * IPV as positive value
• add new parameter "path".
  PowerGateway provides bmsdata under http://hostname:port/BMSData.shtml
  the parameter defaults to "/" so this patch does not break existing configs

[sourcery-ai]

Hey @t3r - I've reviewed your changes - here's some feedback:

• The inline comment for the default path says “usually /BSMData.shtml” but the endpoint is actually BMSData.shtml—please fix the typo to avoid confusion.
• Since both the power and soc sections use the same HTTP URI, consider extracting http://{{ .host }}:{{ .port }}{{ .path }} into a single template variable to reduce duplication.
• You may want to enforce or document that the .path parameter always starts with a slash to prevent broken URIs if someone omits it.

Here's what I looked at during the review

• 🟡 General issues: 1 issue found
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[andig]

What does usually mean? Seems this is nothing a regular user would know? Typical value should be default.

[t3r]

The original code was using the root path, not the BMSData.shtml file. My intention was to keep the original behavior and make this setting optional.
My device does not respond on / but as this is not an official feature supported by solarwatt, I can't say which endpoint is more common.

[andig]

My device does not respond on /

So your device does not work at all with the original tempalte?

[t3r]

Correct, the request returned a 404 and also there was the typo "VBa" which should have been VBat. And even that was the wrong computation as VBat * IBat computes battery power, not PV power.

[andig]

Seems we need to figure out what different devices are out there and split the templates.

[t3r]

I am almost sure that there has never been a "VBa" element and it was just a typo. The relevant part of the JSON response looks like

  "SData": {
      "IBat": -4.33,
      "VBat": 247.7,
      "PGrid": 1,
      "VPV": 370.2,
      "IPV": 5.85,
      "Aux": 0,
      "SysConfig": "19",
      "CCode": "0",
      "Restarts": 0,
      "RelCnt": 1034,
      "Status": {
        "LTC": "00000000",
        "BMS": "00000000",
        "Com": "00000000",
        "Sys": "00000000"
      },

Regarding the URL, my patch keeps the original behavior and optionally allows setting the path.
The official Solarwatt MyReserve wizzard App queries for BMSData.shtml, here is a snipped from my proxy log:
"GET /BMSData.shtml HTTP/1.1" 200 1100 "-" "MyReserve%20Wizard/24 CFNetwork/3826.x.x.x.x Darwin/24.4.0"
Finding out what devices are out in the wild might be tricky. There is no technical documentation publicly available for the MyReserve Command module.

[andig]

Suggested change

	{{- $uri := printf "http://%s:%s%s" .host .port .path }}
	{{- $uri := printf "http://%s:%s/%s" .host .port .path }}

[t3r]

If we add a constant forward slash here, the default path must be an empty string and the path property must not start with a slash to not end up with //.
Probably a matter of taste to force it this way or make it part of the path. I'm happy with both.

[andig]

If we dont it must start wirh slash. ltrim would get it of the slash.

[t3r]

Yes, that's why it is mentioned in the comment :)
Neither ltrim nor trimLeft seems to be available so this does not work:
{{- $uri := printf "http://%s:%s/%s" .host .port (ltrim .path "/") }}
Trigger this:
cannot create meter type 'template': template: base:2: function "ltrim" not defined

[andig]

http://masterminds.github.io/sprig/strings.html