Klipper3d / klipper

Klipper is a 3d-printer firmware
GNU General Public License v3.0
8.99k stars 5.17k forks source link

Further simplifications to host and mcu bulk sensor code #6437

Closed KevinOConnor closed 5 months ago

KevinOConnor commented 6 months ago

This is a continuation of PR #6432 . It is a refactoring of the host and mcu bulk sensor code. The goal is to make it easier to understand the existing "bulk sensors" and to make it easier to support new sensors.

This PR makes changes to the host/mcu messaging system, and thus it will require a reflash of the mcu code. Due to this, I'll look for a merge in mid-January.

This PR is on top of PR #6433.

@garethky, @mattaw - FYI.

-Kevin

KevinOConnor commented 6 months ago

FYI, I added an additional code simplification to this PR - the host python code can initialize the accelerometer chips and the mcu code does not need to schedule starts.

-Kevin

zellneralex commented 5 months ago

@KevinOConnor please use moonlight to anounce the merge at least 1 week before you do it.

moonlight is the right tool to make the user base aware of this “breaking” event.

I am sure @Arksine can help you if you not knowing how to do that announcement with moonlight

KevinOConnor commented 5 months ago

Thanks for the feedback. However, it is "normal" for the MCUs to need to be flashed when updating the Klipper code. So, I don't feel we should make a special announcement for it. If we were to make some kind of special announcement it would need to be something like "always expect to flash the MCUs on every Klipper update".

Cheers, -Kevin

zellneralex commented 5 months ago

Thanks for the feedback. However, it is "normal" for the MCUs to need to be flashed when updating the Klipper code. So, I don't feel we should make a special announcement for it. If we were to make some kind of special announcement it would need to be something like "always expect to flash the MCUs on every Klipper update".

Cheers, -Kevin

Yes for you and me a reflash is something normal. For me it is only an execution of one bash script that updates all 3 mcu’s in my printer. But for the majority of user it is a special event. And after that the helping request will spike as every time the last years.

moonlight is an easy and fast way to make people aware of that the next update will bring a reflash. That would help us people that help people every day to get a working Klipper printer. This time it will get even worse as we do have now many usb adxl modules that will “fail” at the point the user use it next time. So as said the announcement would inform the user base and also would make all the people aware that help in the various discords, groups whatever…

This situation will only get better if we get a more automated way to flash mcu. A feature that you announced in your plans for this year.

KevinOConnor commented 5 months ago

I understand. As noted in the description above, I wanted to give plenty of time to let people know this was coming, and I recently reached out on a couple of forums to let people know this was coming next week. I'm guessing you are writing now because you got word via one of those forums. I also agree that we want to implement automated mcu flashing to make it easier for users.

However, I do not feel comfortable giving widespread notice on mcu changes. In order for us to maintain high quality software and to implement great new features in the future we need the ability to make changes to both mcu and host software, and do so on short notice. So, I don't want to make any kind of announcement that would imply we'll give those types of announcements in the future. If someone wants to get the word out, I feel the message needs to be "every software update could require an mcu flash".

Cheers, -Kevin

zellneralex commented 5 months ago

We at the UI are far ahead of you …

image

There is already are warning implemented, at least in Mainsail.

I generally agree, but to get also a good user experience you need to be more transparent. It is a fact that we might have 1 or 2 updates per year that user force to reflash and the other 100+ nothing happens. I can say for most users it is not understandable and therefore I would use any opportunity to make it more clear.

Regards Alex

TodWulff commented 5 months ago

As a 'non-dev power user', I genuinely look forward to automated MCU flashing. The concept that users should expect that "every software update could require an mcu flash" is unarguably valid. However, if a Host update knowingly requires same, then a better user experience would be had by many if said release is accompanied by an overt heads-up regarding such a requirement.

If there is an opportunity, for a user such as myself, to have input on shaping near-term dev focus, I respectfully request that the automated MCU flashing be an occupant of one of the prioritized focal points for near-term implementation. Thanks for the opportunity to comment.

~MHz

miklschmidt commented 4 months ago

I also agree that we want to implement automated mcu flashing to make it easier for users.

I've had huge success with this in RatOS (which is also why i'd prefer not making every change to mcu code an announcement). Would love an official solution👍

There is already are warning implemented, at least in Mainsail.

Would be super nice if this could be disabled in the settings panel.