esphome / issues

Issue Tracker for ESPHome
https://esphome.io/
290 stars 35 forks source link

Bug remains in Nextion component TFT upload, proposed fixes&PRs blocked twice #4851

Closed andreas-berg closed 1 year ago

andreas-berg commented 1 year ago

The problem

The issue with "reparse mode" described in closed PRs #2956 and #5016, still need fixes.

I wanted to upgrade two of my Nspanels, one with an old version of Blackymas solution and one with joBr99:s solution to the latest Blackymas. Flashing the TFT did not work with any know configuration of the esphome-nextion component. Tried both the forced exit from Reparse in yaml, a dedicated button that appear in newer versions of esphome-configuration from Blackymas as well as the pulling in the PR2956.

I couldn't find the PR5016 available for download, but looking at the code, I think @edwardtfn edwardtfn might have a solution.

@jesserockz declined the PR5016 for the reason that the proposed quickfix breaks the nextion-components basic functionality and cannot be merged.

Could @edwardtfn make a new PR with a parametrized setting(in yaml) of the fix? Which could then be swiftly merged into to the nextion-component. I'd be happy to test the fix if needed.

My workaround involved going back to tasmota32-nspanel.bin and using the berry-component to upload Blackymas TFT-file, worked like a charm but it is obvious and humiliating that the old berry-implementation is so superior to esphome:s nextion-component.

Which version of ESPHome has the issue?

all versions

What type of installation are you using?

Docker

Which version of Home Assistant has the issue?

not HA issue

What platform are you using?

ESP32

Board

Sonoff NSPanel

Component causing the issue

esphome/components/nextion

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

randybb commented 1 year ago

I have been using my nspanels with https://github.com/sairon/esphome-nspanel-lovelace-ui (it is based on the original nspanel component) never had any issues with flashing, so probably this is the way how to fix that.

andreas-berg commented 1 year ago

@randybb, I also use sairons code on my nspanel-lovelace-ui-esphomes but I don't like the appDaemon dependency in nspanel-lovelace-ui. I have to maintain a separate docker just running the appDaemon-server and I'd like to get rid of that by using the Blackymas-templates. This "reparse mode"-issue exists apparently only with the Blackymas-templates but to my understanding it is a real issue, pls correct me if it's just something that Blackymas is doing wrong. Perhaps one solution would be to override the TFT-Upload service in the Blackymas-yaml with the same service from sairon, but that seems to be an overkill.

edwardtfn commented 1 year ago

I wouldn't call this a bug, as it was intentionally made this way. And I understand the reason for not implementing the proposed solution, as that will break any project where reparse mode is desired, although I don't believe they are that many, if any. What I don't like is the fact that the development team don't pay attention to this issue, despite the huge majority of users being affected.

That PR I've created was just an update for #2956 from @maretodoric, as the older code was failing compiling with latest ESPHome.

Anyways, there are some suggestions on the discussion area of #2956, however that didn't worked for me in a reliable way, therefore I still using these PRs as reference.

I like the idea of having settings to enable reparse mode. I will see if I can find some time to look at that. Or hopefully someone else can do it.

andreas-berg commented 1 year ago

I'm starting to suspect that my original issue has nothing to do with this whole reparse-on/off-business. When doing some dev around the Blackymas-solution, I get the same TFT-upload failures when there is a mismatch in baudsetting between esphome-yaml and the baudsetting embedded in the currently running TFT.... obviously

I understand why the esphome devteam didn't want to merge #2956 and #5016 since both hardcodes the "reparse off"-state for all devices using the nextion-module in esphome. Perhaps there is a valid use-case for "reparse on", I don't know, so if someone would do a PR that is compatible with both states, then that should not be a problem to merge (I hope).

So afaics there is no bug anymore and I'll close this issue.

edwardtfn commented 10 months ago

https://github.com/esphome/esphome/pull/5868