Web pendant fixes

[LepeshkinSN]

Brief Fixed some problems with web pendant:

1. Wrong (only on Linux?) HTTP content-length value while serving disk-sourced content (canvas and icons)
2. Problems with file upload (seems to be related to Python 3)
3. Complex canvas structure on slow machines cause refresh failure
4. Infinite growth of canvas image cgi argument string

Details This changes addresses issue #1509 (and maybe some others). I'am using bCNC 0.9.14 on Debian 11 x64 with Python 3.9.2. My browser is [Version 1.31.91 Chromium: 95.0.4638.69 (Official Build) (64-bit)]. Web pendant was not usable for me because there were no canvas image (but sometimes it flickers) and I wasn't able to upload file through it. Here is related problems and fixes:

1. It seems os.path.getsize() on my system returns disk size of file, not content length of file. So, browser gets wrong Content-Length size on small files (smaller than disk block size) and discards it content after connection closing. I've replaced getsize() with seek()/tell() combinations.
2. File upload code seems to be not adapted to Python3. There are some changes in classes and between bytes/strings arguments and variables.
3. When your cnc code is very long, it tooks a while to generate canvas image. On timeout (1 sec.), old code causes request abortion and generation of the new request, which invoke new canvas generation and this sequence will never stop, and you will see no canvas at all. My changes adds check whether previous canvas image was loaded before starting new request.
4. Canvas image update code adds current time as argument to canvas image url. Maybe it's bug in browser, but in such case URL of canvas image grows infinitely. I've hard-coded pre-argument part of URL and it seem's to work.

[rar8000]

I merged these changes manually to a local clone of the current release and to my PR. It works great. For large images, the pendant does not show images without these changes.

Thanks!

PS: Since this work was done before the python 3.8 port. I had to do the merge manually in an editor.

[Harvie]

It works great

Thanks for the review

Unfortunately i cannot merge this PR, since there's conflict

[LepeshkinSN]

Hi! Could you explain what conflict it is? There were problem with codefactor check, but now it's resolved. But it seems there are some other conflicts. On "Resolve conflicts" page there are some chunks of code, but they differ only in indention size. Payload is the same. What's wrong with it?

[Harvie]

It's not really about codefactor. it's simply the fact that some of the files you are trying to modify had been also changed in vlachoudis/bCNC since you've forked and therefore the changes in your fork cannot be applied, you need to merge vlachoudis/bCNC first and manualy resolve the conflicts in order to not overwrite changes done in upstream by other PRs...

[LepeshkinSN]

Hmm... If so, why it was not merged a year ago, when I've created this pull request? My repository was cloned and modified the same day. Furthermore, next changes to Pendant.py are introduced only at 1 Sep. 2022, while my pull request is from 23 May 2022. BTW, at that time, there were codefactor check fail, but it refers to original chunk of code. There were no check fails in my changes. I guess story is this: a year ago I've made changes to original code. At this moment original code were not conform to codefactor, but codefactor checks for pull requests were already on. Because of that my pull request were not merged automatically and left for manual merge by repository owners. Unfortunately, they didn't manage to merge it in time. May be they even didn't look on it. Instead of this, they introduced some code cleanup and prettier. And now there is conflict between old code and new "pretty" one. Well, ok. Now I have no time to deal with this new pretty code and you have to do it by yourself. Cheers!

[Harvie]

Maybe there were more PRs affecting the web pendant. i merged the first one and this one become conflicted immediately, therefore i've waited for a year if you notice. I understand your frustration, but that's just how things are when multiple people create PRs affecting same files. However it seems this conflict might not be really hard to resolve, since the changes are reasonably small.

Maybe @rar8000 can give us some insight how he got this merged when he's been testing it.

[rar8000]

Hi. Basically I went line by line and copy pasted. The changes don't conflict to the human eye, but the line numbers have been shifted; so git cannot merge. I cloned a fresh updated bCNC, then manually found each line via github interface. Then copy and pasted. Took less than 5 mins. I attached the two updated files in a zip.

pendant.zip

[rar8000]

I also merged the changes to my "test" branch. Different than my release branch. This fix gets the pendant working for me.

Other errors are still thrown when the pendant is running, but they seem to be caught and displayed. These errors are separate from this fix. With the fix enabled, the pendant works, though a bit slow and can periodically hang the UI updates to the main program while running.

[LepeshkinSN]

Added Harvie modifications and finally it can be merged.

[Harvie]

There still seems to be some conflict preventing me from merging this...

[LepeshkinSN]

There still seems to be some conflict preventing me from merging this...

I can't help you in this situation - GitHub shows me "Able to merge. These branches can be automatically merged.", "All checks have passed", "This branch has no conflicts with the base branch". All green marks.

[Harvie]

I can't help you in this situation - GitHub shows me "Able to merge.

oh sorry, when i switch from "rebase" to "merge". it's suddenly green.

[Harvie]

Weird, i've tried this:

import os

print(os.path.getsize("test.jpg"))
print(os.stat("test.jpg").st_size)

and both do return same number as wc -c test.jpg. perhaps it's some issue specific to your version of pyhton...

[Harvie]

Maybe you can also try os.fstat(fd).st_size which works directly on the open file descriptor rather than path...

[LepeshkinSN]

Yes, I've checked os.path.getsize() - now it returns correct value. Maybe it's better to use this function instead of my workaround. But at the time I created this pull request it was way I could do it function properly.

ср, 14 февр. 2024 г., 14:45 Tomas Mudrunka @.***>:

Maybe you can also try os.fstat(fd).st_size which works directly on the open file descriptor rather than path...

— Reply to this email directly, view it on GitHub https://github.com/vlachoudis/bCNC/pull/1705#issuecomment-1943610702, or unsubscribe https://github.com/notifications/unsubscribe-auth/AH4O5Y36O3GRPCLD57U4I5DYTSPW3AVCNFSM5WVHTE2KU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOJUGM3DCMBXGAZA . You are receiving this because you authored the thread.Message ID: @.***>

[Harvie]

Yes, I've checked os.path.getsize() - now it returns correct value.

now i am quite confused. i thought that you were doing this workaround, because os.path.getsize() is not working for you... what had changed in the meantime? have you upgraded your python version or something?

[LepeshkinSN]

I think minor version number of Python 3 on my system was changed since then. Periodically I perform system update from standard Debian repo, and it upgrades pretty much packages. So it is very likely Python 3 was among them.

ср, 14 февр. 2024 г., 20:15 Tomas Mudrunka @.***>:

Yes, I've checked os.path.getsize() - now it returns correct value.

now i am quite confused. i thought that you were doing this workaround, because os.path.getsize() is not working for you... what had changed in the meantime? have you upgraded your python version or something?

— Reply to this email directly, view it on GitHub https://github.com/vlachoudis/bCNC/pull/1705#issuecomment-1944264995, or unsubscribe https://github.com/notifications/unsubscribe-auth/AH4O5YZAMGFXNUTKNX3REXLYTTWMJAVCNFSM5WVHTE2KU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOJUGQZDMNBZHE2Q . You are receiving this because you authored the thread.Message ID: @.***>