Skip to content

Conversation

@DavidvtWout
Copy link
Contributor

@DavidvtWout DavidvtWout commented Nov 7, 2025

I wanted to set pwm_cycle_time to 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_time is 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_time option for the SPI sensors. But adding a report_time option to all sensor would probably be preferred?

@nefelim4ag
Copy link
Collaborator

nefelim4ag commented Nov 7, 2025

I'm aware of what you're talking about: https://klipper.discourse.group/t/ssr-and-klipper-pwm/18238
This your change should work correctly.

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:

  • Rare reports - some are missing (packets are missing) in the 3s window - shutdown because there was no PWM update scheduled.
  • Faster reports - it will schedule closer in time, and it is easier to get the TTC error.

I'm not sure it is worth it, as it would create additional failure modes.

Hope that explains something.

Copy link

@MaricanEngineering MaricanEngineering left a 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!

Comment on lines 2909 to 2911
#tc_report_time: 0.3
# The interval, in seconds, between temperature queries sent from
# the MCU to the sensor.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to config

Comment on lines 24 to 25
self._report_time = config.getfloat(
"tc_report_time", default=0.3, minval=0.1)

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

Comment on lines -42 to +44
return REPORT_TIME
return self._report_time

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

Comment on lines -48 to +50
self._report_clock = self.mcu.seconds_to_clock(REPORT_TIME)
self._report_clock = self.mcu.seconds_to_clock(self._report_time)

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

@DavidvtWout
Copy link
Contributor Author

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.)

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.

Also, this sort of change makes it possible to narrow the control window and can increase the possibility of different errors, for example:

* Rare reports - some are missing (packets are missing) in the 3s window - shutdown because there was no PWM update scheduled.

* Faster reports - it will schedule closer in time, and it is easier to get the TTC error.

I'm not sure it is worth it, as it would create additional failure modes.

I have been running at pwm_cycle_time=2.0 for some months now and haven't experienced issues yet (I know, anecdotal evidence doesn't really count ;) ).

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'm aware of what you're talking about: https://klipper.discourse.group/t/ssr-and-klipper-pwm/18238 This change should work correctly.

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.
PID can sort of handle this but it might react very differently at different duty cycles. Since it is calibrated for a specific temperature region it might just not work at all at lower temperatures for example.

image

@DavidvtWout DavidvtWout mentioned this pull request Nov 10, 2025
@DavidvtWout
Copy link
Contributor Author

DavidvtWout commented Nov 12, 2025

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 tc_report_time would cause issues.

Around tc_report_time: 0.007 the Timer too close error starts to appear. This was tested with pwm caching disabled to send as many pwm updates as possible. The test setup was a raspberri py 4 with the MCU also running on the same pi using the gpio SPI pins. So a tc_report_time minimal value of 0.1 seems okay.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants