Skip to content

JSON Parsing Causes Dynamic Page Woes #500

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
DropTarget opened this issue Jun 14, 2022 · 8 comments
Closed

JSON Parsing Causes Dynamic Page Woes #500

DropTarget opened this issue Jun 14, 2022 · 8 comments
Labels
bug Something isn't working enhancement New feature or request fixed The issue has been resolved

Comments

@DropTarget
Copy link

I've been struggling to get a dynamic page operating correctly and finally found the root cause of my problem. The "Communicate with the Sketch using XHR" section of the documentation shows a simple example of updating a time display on the client web page. However, when using the example as written, the JSON parser creates HTML without an ID for the 'time' element. I now realize that this is probably due to the issue described in the note "AutoConnect JSON parsing process is not perfect" in the "Custom Web pages with JSON" section. I was able to determine that no ID was created by using inspection mode in the browser. Simply adding a "style" attribute to the 'time' element causes the JSON parser to create an ID and the script works properly.

I was wondering if there might be a way to make this more clear so that others don't encounter the same issue and need to go through a debug process to figure it out. Would it be possible to add a directive that would tell the JSON parser not to optimize? Maybe this already exists and I'm not aware of it.

Thanks again for a great library.

@Hieromon
Copy link
Owner

Hieromon commented Jun 15, 2022

@DropTarget Thank you for your suggestion.

It seems that you have discovered the hidden degradation caused by AutoConnect enhancement. My lack of consideration caused it.
As you noted, in the past, I have made enhancements to AutoConnect's HTML generation process for custom web pages to exclude HTMLing of AutoConnectText with AC_Tag_None attributes that have no value, style, and format. As a result, HTML generation is missing for "elements that are not needed initially but require activation during the rendering process," as the XHR clock example shows.

To make matters worse, an id is generated only if the style is specified. This is clearly an inconsistent specification.

The current AutoConnectText HTML element generation conditions are as follows, but I would like to determine a more consistent specification through this topic.

`style` presented `style` not presented
`value` presented `value` not presented
AC_Tag_None AC_Tag_BR AC_Tag_P AC_Tag_DIV
Generated HTML <div id='name' style='style'>Formatted value</div> Formatted value Formatted value<br> <p>Formatted value</p> <div>Formatted value</div> Not generated

My candidate spec would be to generate an element with an id even if no style is provided. It would probably be better to apply <span id='name'>Formatted value</span> or <p id='name'>Formatted value</p>. The resulting HTML is expected to look as follows:

  • AC_Tag_None: <span id='name' style='style'>Formatted value</span>
  • AC_Tag_BR: <span id='name' style='style'>Formatted value</span><br>
  • AC_Tag_P: <p id='name' style='style'>Formatted value</p>
  • AC_Tag_DIV: <div id='name' style='style'>Formatted value</div>

These HTML are generated in a fixed manner, regardless of whether value or style is provided.
Please let me know your opinion to improve the above specifications. Thank you for your contribution.

EDIT
I have staged a fix based on the above tentative specification to the enhance/v135 branch. Please evaluate if possible.

@Hieromon Hieromon added bug Something isn't working enhancement New feature or request labels Jun 15, 2022
Hieromon added a commit that referenced this issue Jun 15, 2022
Hieromon added a commit that referenced this issue Jun 16, 2022
@DropTarget
Copy link
Author

@Hieromon
This looks like a good solution. While it produces a larger html footprint, it provides more consistency and removes the necessity to inspect the resulting html when things don't work as expected, like performing a getElementById when no id was generated.
I'll try out the candidate and let you know...

@DropTarget
Copy link
Author

DropTarget commented Jun 18, 2022

@Hieromon
I believe that this issue is also related to #482, where AutoConnectUpdate hangs when using an mDNS name to access the server during update.
In AutoConnectUpdateAct::_elmProgress, the { AC_Text, "status", nullptr, nullptr } entry is produced without an ID. The effect of this is seen when the script function bar() is executed. The XHR to start the update progress has a timeout of 8 seconds. The ESP32 takes longer than this to respond to mDNS requests. When the timeout occurs, the intent of the bar() function is to display a "Could not start" message. But the getElementById("status").textContent fails due to the missing ID. If the ID had been present, we would have seen the error message and pinpointed the source of the problem earlier. Increasing the timeout in the bar() function fixes the problem since the mDNS has enough time to resolve the address.
So, this update of html generation might resolve a number of issues.

Thanks

EDIT: Further investigation reveals that the delay in mDNS is not caused by ESP32. In fact, the ESP32 immediately responds to the mDNS broadcast on the local network. The delay appears to be in Windows 10, due to use of a VPN tunnel. mDNS brodcasts are routed through the tunnel as well as on the local network. This seems to cause a delay of about 10 seconds before Windows acts on the response - probably a timeout waiting for a response from the VPN tunnel. Disconnecting the VPN results in normal mDNS operation.

@DropTarget
Copy link
Author

@Hieromon
I am currently testing enhance/v135 and am experiencing truncated AC_Text strings. I have determined that the fault is in AutoConnectElementBasisImpl.h. At line 481, the calculation of elmLen results in too short a length due to the use of sizeof(elmTextTemp). The compiler returns a size of 4U (pointer size) instead of the actual literal count (I counted 10 decimal) in the format string. By using a separately defined constant of 10 instead of sizeof(elmTextTemp), the issue is resolved and all looks good. It's nice to depend on having an ID on each element.

@DropTarget
Copy link
Author

@Hieromon
Same issue with style builder at line 472, using sizeof(attrStyleTemp). I used a literal 9 and works fine.

@Hieromon
Copy link
Owner

@DropTarget Thank you for your testing.

The compiler returns a size of 4U (pointer size) instead of the actual literal count (I counted 10 decimal) in the format string.

That's exactly. It is my careless mistake. I'm aware of that but I'm working on changing the HTML generation logic from the current approach of concatenating String classes to one based on syntax templates. It will reduce the flashed size of the compiled bin. I should be able to stage the fix into the enhance/v135 branch within a few days. Also, the fix of the OTA Update script that AutoConnectUpdatePage.h has is also expected to be completed together.
I will post to inform you after I stage the fix. Thank you for your contribution.

@Hieromon
Copy link
Owner

@DropTarget I have brushed up the fix to an appreciable and I think you can give it a try on enhance/v135 branch. There may still be minor bugs lurking. If you find it I would be of great help.

Also, thank you for the heads-up on the mDNS issue. I have not yet fully examined the information you provided. I will start working on it and will let you know if anything turns up.

@Hieromon Hieromon mentioned this issue Jul 2, 2022
@Hieromon
Copy link
Owner

Closed due to the release of v1.3.5

@Hieromon Hieromon added the fixed The issue has been resolved label Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request fixed The issue has been resolved
Projects
None yet
Development

No branches or pull requests

2 participants