spi_flash: Support Creality CR-FDM-v2

[mnorrsken]

Supports Creality CR-FDM-v2 boards. Possible to use unique filenames based on firmware hash by using %%HASH%% codeword in filename

[JamesH1978]

Thankyou for submitting a PR, please can you fix the whitespace errors and line lengths so the check pass.

  ERROR:
  docs/SDCard_Updates.md:140: Line has trailing spaces
  scripts/spi_flash/spi_flash.py:34: Line longer than 80 characters
  scripts/spi_flash/spi_flash.py:35: Line longer than 80 characters
  scripts/spi_flash/spi_flash.py:38: Line longer than 80 characters
  scripts/spi_flash/spi_flash.py:[13](https://github.com/Klipper3d/klipper/pull/6207/checks#step:5:14)94: Line has trailing spaces
  scripts/spi_flash/spi_flash.py:1396: Line has trailing spaces```

Thanks
James

[KevinOConnor]

Thanks. I don't think I'm in a good position to review this PR. Maybe @Arksine or @JamesH1978 can help.

-Kevin

[JamesH1978]

more @Arksine territory, would be nice to have unique filenames to sort the issue with boards that take a different flash name every time. Not sure if there was issue doing this, I do seem to remember it being mentioned when we last looked at the creality boards that are currently in the script

Thanks James

[Arksine]

I believe I had previously approved these changes in another pull request, however as I recall there were merge conflicts due to the SDIO addition. I'll take another look at the changes over the next day or so just to be sure.

@JamesH1978 I don't think I was privy to this discussion, however I agree that it sounds like a change like this is necessary to support creality boards. That said, I don't understand why board manufacturers aren't willing to stick to an established standard. Its as if they just desire to be unique without any specific gain.

[JamesH1978]

creality is, as creality does... I am sure there was a PR mentioning it before, or i'm mistaken and it was a random chat :) but unique name generation would be good to make sure the tool works more than once.

[mnorrsken]

Yes I had a PR before that broke because of refactoring of the IO stuff, so I made a new one. The only thing I am not sure of is the ”templating” maybe it is over thought and should be a boolean setting in board defs instead, something like ”creality_quirks”.

fre 26 maj 2023 kl. 08:49 skrev JamesH1978 @.***>:

creality is, as creality does... I am sure there was a PR mentioning it before, or i'm mistaken and it was a random chat :) but unique name generation would be good to make sure the tool works more than once.

— Reply to this email directly, view it on GitHub https://github.com/Klipper3d/klipper/pull/6207#issuecomment-1564343853, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALCB73ARRFU7ZI2HV5PAM5DXICRGRANCNFSM6AAAAAAYFURXIY . You are receiving this because you authored the thread.Message ID: @.***>

[Arksine]

Thanks. I had a couple of minor observations, however overall it looks good.

Yes I had a PR before that broke because of refactoring of the IO stuff, so I made a new one. The only thing I am not sure of is the ”templating” maybe it is over thought and should be a boolean setting in board defs instead, something like ”creality_quirks”.

We could go with a boolean option rather than the template. The firmware_path could be a subfolder joined to the procedurally generated filename. Something like requires_unique_filename, although that may be a bit verbose. I would stay away from anything referencing Creality specifically for the option as its possible another manufacturer will use the same pattern.

[KevinOConnor]

Thanks. What is the current status of this PR?

-Kevin

[github-actions[bot]]

Thank you for your contribution to Klipper. Unfortunately, a reviewer has not assigned themselves to this GitHub Pull Request. All Pull Requests are reviewed before merging, and a reviewer will need to volunteer. Further information is available at: https://www.klipper3d.org/CONTRIBUTING.html

There are some steps that you can take now:

1. Perform a self-review of your Pull Request by following the steps at: https://www.klipper3d.org/CONTRIBUTING.html#what-to-expect-in-a-review If you have completed a self-review, be sure to state the results of that self-review explicitly in the Pull Request comments. A reviewer is more likely to participate if the bulk of a review has already been completed.
2. Consider opening a topic on the Klipper Discourse server to discuss this work. The Discourse server is a good place to discuss development ideas and to engage users interested in testing. Reviewers are more likely to prioritize Pull Requests with an active community of users.
3. Consider helping out reviewers by reviewing other Klipper Pull Requests. Taking the time to perform a careful and detailed review of others work is appreciated. Regular contributors are more likely to prioritize the contributions of other regular contributors.

Unfortunately, if a reviewer does not assign themselves to this GitHub Pull Request then it will be automatically closed. If this happens, then it is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available.

Best regards, ~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

[mnorrsken]

Thanks. What is the current status of this PR?

-Kevin

I'm not sure... I changed it so that it is a boolean option and.. well thats it I guess. Tested on my S1 pro. I'm only hanging here when I'm "in the zone", I thought this was gonna be accepted already :)

[Arksine]

I left a couple of comments in a review above that haven't been addressed. Specifically, now that SDIO is supported on the F4 we should avoid using Software SPI on new configurations. That way the entire upload/verification process can be done at once.

[mnorrsken]

Oh ok, I don't know what the SDIO thing is, I have to read into that then. But I noticed another problem too so dont merge this until I fixed it, it seems there can be no other .bin files in the folder. Or there is something else going on. Anyway, if I run the update two times the second time it doesn't read the bin file. If I have to remove the card to clear the folder, this patch is kind of useless.

[github-actions[bot]]

Unfortunately a reviewer has not assigned themselves to this GitHub Pull Request and it is therefore being closed. It is a good idea to move further discussion to the Klipper Discourse server. Reviewers can reach out on that forum to let you know if they are interested and when they are available.

Best regards, ~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

[JamesH1978]

Just resurrecting this necro-pr.

@mnorrsken Did you ever get anywhere with this?

@Arksine This is still an issue with the stock boards like creality that need a unique file name every time and cannot have more than one bin in the directory. Is there any way we can clean this up so we can have the functionality and the ability to clear the contents of the card first?

Thanks

James

[Arksine]

It should be possible to clear the contents of the directory first, the functionality to list directory contents and remove items is in there. Such functionality would likely require adding another field to the board definitions.

I do have a yet to be submitted PR that improves verification. Rather than reading out the .CUR file from the SD card it uses Klipper's debug_read command to directly compare the contents of a binary to the contents flashed. This functionality would make implementing the "random file name" easier, as it wouldn't be necessary to generate it from a hash, nor would it be necessary to know what the file name is during validation.

In the near term I'm a bit strapped for time, but I will look into it when I get a chance. You might need to nudge me again in the future.