ph1p / ikea-led-obegraensad

ESP32/Arduino hack for the ikea OBEGRÄNSAD led wall lamp
MIT License
579 stars 78 forks source link

Make it clearer the existing webgui.cpp contains a (probably) wrong IP #7

Closed ghost closed 1 year ago

ghost commented 1 year ago

Great project, but I think the setup instructions regarding that could be a bit clearer, it's as far as I can tell only mentioned in the Development section of the readme which I glossed over initially because I didn't think it was relevant to setup, especially since the webgui.cpp it says it creates already did exist. If you forget to do this it will just hang on connecting/try to reconnect on access on the webui.

Alternatively is there maybe a way to make the frontend installed on the device be agnostic to the IP?

VincentSC commented 1 year ago

The problem is here:

    `${
      (import.meta as any).PROD
        ? location.href.replace('http', 'ws')
        : import.meta.env.VITE_WS_URL
    }ws`

in files app.tsx and Creator.tsx. The (import.meta as any).PROD gives false, while it should give true in production. Also, I think it should use window.location.href.

This fixes it:

    `${
      import.meta.env.PROD
        ? window.location.href.replace('http', 'ws')
        : import.meta.env.VITE_WS_URL
    }ws`

Search for ViteJS import.meta.env.PROD to understand what happens here in more detail. In short: the data from .env is only used when running npm run dev.

TBH, I think it should always use window.location.href.replace('http', 'ws') in the current state of development.

VincentSC commented 1 year ago

I just started today with the project, and since configuration is done in-code, I cannot easily create a PR. I first need to decouple that, so I don't accidentally share sensitive info.

VincentSC commented 1 year ago

Ok, got the lines replaced with:

`${window.location.href.replace('http', 'ws')}ws`,

This will always use the current server.

letalvoj commented 1 year ago

I added a mDNS answering to obegraensad.local, similarly to how Raspbian answers to raspberrypi.local, which solve this issue for me.

Not sure if it is worth upstreaming. Let me know!

VincentSC commented 1 year ago

Thanks for accepting my "PR"