Skip to content

Asynchronous conversion? #11

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.

Sign up for GitHub

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

Jump to bottom
Closed
LBertrandDC opened this issue Jun 29, 2019 · 5 comments
Closed

Asynchronous conversion? #11

LBertrandDC opened this issue Jun 29, 2019 · 5 comments

Comments

@LBertrandDC
Copy link

The DS18x20 temperature conversion is quite slow, up to 750ms for 12 bit resolution and the application is unresponsive until the conversion is complete. My enhancement request is for an asynchronous conversion, much like the Arduino DallasTemperature library by Miles Burton et al.

It works something like this (I'm using the RBD::Timer library for timers to avoid delay() ):

void setup() {
    /* Start up the DallasTemperature library
     * IC Default 9 bit. If you have troubles consider upping it 12.
     * Ups the delay giving the IC more time to process the temperature measurement
     * (Miles Burton)
     * The conversion is very slow (approx. 750ms). As a workaround, the 
     * temperature conversion command is sent to all devices on the bus 
     * but the program thread is not blocked. Later, the temperatures are
     * read back one device at a time. 
     */
    sensors.begin(); 
    // Do not block during conversion (read later)
    sensors.setWaitForConversion(false);
}

void loop() {
    /* The temperature converstion takes approx. 750ms,
     * therefore the conversion is handled asynchronously.
     * The conversion is started but the program is not 
     * blocked. Later, the program gets each temperature.
     * This is a simple state machine but not implemented
     * with a formal structure.
     */
    if(thermo_timer.onRestart())
    {
        // Issue a global conversion command to all ICs on the bus
        sensors.requestTemperatures();
        thermo_conv_in_progress = true;
        thermo_conv_timer.restart();
    }

    if(thermo_conv_in_progress && thermo_conv_timer.onExpired())
    {
        thermo_conv_in_progress = false; // mark complete
        // Why "byIndex"? You can have more than one IC on the same bus.
        // 0 refers to the first IC on the wire (Miles Burton).
        temp_reading_heatsink = sensors.getTempCByIndex(HEATSINK_THERMOMETER);
        temp_reading_case = sensors.getTempCByIndex(CASE_THERMOMETER);
    }
}

I would like to help implement this but I'm quite new at CircuitPython, Git, GitHub and all that. Before starting, I would like to better define an API addition that doesn't break previous code while keeping with the learner friendly mandate of CircuitPython: keep the temperature property and add a method to kick off the conversion.

Perhaps something like this:

# keyword argument to wait, defaults to waiting
DS18X20(bus, address, wait_for_conversion=True)

    def request_temperature(self):
        # Similar to _convert_temp(self) but without the timeout check
        with self._device as dev:
            dev.write(_CONVERT)
            # EXPECTED_CONVERSION_DELAY need not be a constant,
            # it can be based on resolution as given in the datasheet.
            return time.monotonic() + EXPECTED_CONVERSION_DELAY

    @property
    def temperature(self):
        """The temperature in degrees Celsius."""
        # Caveat utilisator: if you cleared the flag and check too soon, 
        # the temperature is invalid.
        # Alternatively the bus or device can be polled to check for completion.
        # The method would still block if the routine didn't wait long enough,
        # but you would never get an invalid temperature.
        if self.wait_for_conversion:
            self._convert_temp()  
        return self._read_temp()

Thoughts? Where's the best place to discuss this? (I'm not on Discord but I can PM you my work email.)
@ladyada
Copy link
Member

ladyada commented Jul 2, 2019

hiya i think it would be best to add two new functions - one for start_temperature_read and one for read_temperature that are functions - leave the temperature property as is, and advanced folks can use the 2 functions

@LBertrandDC
Copy link
Author

LBertrandDC commented Jul 2, 2019 via email

@LBertrandDC
Copy link
Author

LBertrandDC commented Jul 31, 2019 via email

@ladyada
Copy link
Member

ladyada commented Jul 31, 2019

hiya you should commit your changes to your fork, then make a PR
see https://learn.adafruit.com/an-introduction-to-collaborating-with-version-control

@LBertrandDC LBertrandDC mentioned this issue Aug 8, 2019
Merged
@tannewt
Copy link
Member

tannewt commented Aug 12, 2019

Fixed by #12. Thanks @LBertrandDC !

@tannewt tannewt closed this as completed Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@tannewt @ladyada @LBertrandDC