-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
spi_temperature.py: add tc_report_time option #7114
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
base: master
Are you sure you want to change the base?
Conversation
|
I'm aware of what you're talking about: https://klipper.discourse.group/t/ssr-and-klipper-pwm/18238 But, isn't the 0.2+ enough for what you are trying to accomplish? (It still would flicker if there is a bad light or weak power lines.) But generally, the way to work around it is to increase the frequency for some reason, probably because it still can close open when the voltage is above zero with some margin, and this can spread out the load spikes. Also, this sort of change makes it possible to narrow the control window and can increase the possibility of different errors, for example:
I'm not sure it is worth it, as it would create additional failure modes. Hope that explains something. |
MaricanEngineering
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been looking for a change like this!
docs/Config_Reference.md
Outdated
| #tc_report_time: 0.3 | ||
| # The interval, in seconds, between temperature queries sent from | ||
| # the MCU to the sensor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding definition and moving from separate spi config file
| # SensorBase | ||
| ###################################################################### | ||
|
|
||
| REPORT_TIME = 0.300 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to config
klippy/extras/spi_temperature.py
Outdated
| self._report_time = config.getfloat( | ||
| "tc_report_time", default=0.3, minval=0.1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing to be more descriptive
| return REPORT_TIME | ||
| return self._report_time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing to be more descriptive
| self._report_clock = self.mcu.seconds_to_clock(REPORT_TIME) | ||
| self._report_clock = self.mcu.seconds_to_clock(self._report_time) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing to be more descriptive
0.3s should be okay, but a higher cycle time wouldn't hurt. A heating bed has a high heat capacity and is slow to react so a higher cycle time is okay. This would run the SSR slightly cooler which might even extent its lifespan.
I have been running at If every 3 seconds a PWM update must be scheduled, would it make sense to limit the pwm_cycle_time to 1.5s to allow for a single missed package?
I read the discourse thread and I think a warning message should be added for future readers. Since the thread is closed I can't react to it anymore. Syncing the pwm_cycle_time with the grid may "feel right" but it's actually a very bad idea! Because the SSR turns off at the same moment of every oscillation, you are modifying the waveform of the AC power in your house. With a sufficiently large load this can become a serious problem. Another issue with this approach is that control becomes non-linear since the first part of the wave has a lower voltage and wouldn't heat up the bed as much as the peak of the wave.
|
Signed-off-by: David van 't Wout <[email protected]>
2aad8fa to
7de9984
Compare
|
Something I did not realise earlier is that the report time of the sensor is also used as a pwm delay for the heaters. So today I did some testing to see if a lower Around I also checked the actual update interval of my MAX31856 sensor (I assume there isn't much variance in update time between different chips?). It's somewhere around 99 ms and setting the report time to 100 ms guarantees that the sensor always has a new value. |

I wanted to set
pwm_cycle_timeto a somewhat higher value than the default (0.1s) since the low cycle time in combination with a zero-crossing SSR caused a slightly noticable flickering of my lights. However,pwm_cycle_timeis not allowed to be set to a higher value than the REPORT_TIME of the temperature sensor, which is 0.3s. I would like to be able to increase this value in the config file.Another advantage is that people can set a much lower report time for a better response time. This could probably be useful for hotends with a very low heat capacity.
This PR only adds a
tc_report_timeoption for the SPI sensors. But adding a report_time option to all sensor would probably be preferred?