Skip to content

Conversation

@stoklund
Copy link
Contributor

When converting a fan speed percentage to an LSB byte written to the EMC2101 controller, the conversion should translate 100% to 2*PWM_F because the chip determines the PWM duty cycle that way.

The changes in this PR calculate and cache the byte value to use for 100% fan speed in the function _calculate_full_speed(). This cached full speed value is recomputed whenever emc.pwm_frequency or emc.dac_output_enabled are changed — these are the two registers affecting the value.

When setting emc.manual_fan_speed or one of the LUT values, the cached full speed value is used to compute the byte to be programmed into the chip. This means that emc.pwm_frequency should always be set before programming the LUT or manual speed setting.

My new oscilloscope arrived today (yay!), so I was able to test this code by measuring the duty cycle on the FAN pin. I tested 10% fan speed increments both with the default PWM frequency, and with PWM_F=5 which produces a 35 kHz PWM signal.

pwm_f5.pdf

Both PWM frequencies plot as a straight line. At 35 kHz, I measured a duty cycle about 0.75% less that the programmed value. I think this is caused by the 1 µs rise time on the open drain FAN pin.

This was previously fixed in adafruit#4, but it looks like the fix got clobbered when the LUT code was split out to a separate file.
This conversion is going to depend on the PWM_F and DAC register values, so it needs to be a method. Use this method in `manual_fan_speed()` as well as the LUT subclass. Move the `_pwm_freq` register definition into the base class too. It will be needed for the fixed fan speed conversions. Also import the `MAX_LUT_*` constants from the base module rather than redefining them.
- Add a `_full_speed_lsb` member variable which represents the (smallest) LSB value corresponding to a 100% fan setting. - Add a `_calculate_full_speed()` method that computes it. - Use `_full_speed_lsb` when converting percentages to fan speed settings and back. Fixes adafruit#19.
Recalculate the full speed LSB setting when this property is changed too.
@ladyada ladyada requested a review from a team November 1, 2021 19:13
@ladyada
Copy link
Member

ladyada commented Nov 1, 2021

thanks! hopefully someone from the circuitpy community could test this PR and then we will merge

@askpatrickw
Copy link

@stoklund is this a fix for #19 ?

@stoklund
Copy link
Contributor Author

stoklund commented Nov 6, 2021

@stoklund is this a fix for #19 ?

Yes

Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

I tested this branch successfully with a Feather RP2040 + EMC2101 + a 5v Raspberry Pi cooling fan. All examples in the repo seem to work as expected.

@FoamyGuy FoamyGuy merged commit 43cc811 into adafruit:main Jan 10, 2022
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jan 10, 2022
Updating https://github.com/adafruit/Adafruit_CircuitPython_DPS310 to 2.1.0 from 2.0.0: > Merge pull request adafruit/Adafruit_CircuitPython_DPS310#20 from FoamyGuy/default_import Updating https://github.com/adafruit/Adafruit_CircuitPython_EMC2101 to 1.1.10 from 1.1.9: > Merge pull request adafruit/Adafruit_CircuitPython_EMC2101#20 from stoklund/full_speed > update rtd py version Updating https://github.com/adafruit/Adafruit_CircuitPython_HT16K33 to 4.1.9 from 4.1.8: > Merge pull request adafruit/Adafruit_CircuitPython_HT16K33#95 from tekktrik/doc/update-documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants