Skip to content
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

Compatibility with Genuine MicroPython #10

Closed
amotl opened this issue Nov 17, 2019 · 7 comments
Closed

Compatibility with Genuine MicroPython #10

amotl opened this issue Nov 17, 2019 · 7 comments

Comments

@amotl
Copy link
Contributor

amotl commented Nov 17, 2019

Hi there.

This module is coming from a time when neither Genuine MicroPython nor Pycom MicroPython had native drivers for 1-Wire/DS18X20 sensors so they had to be implemented in pure-Python.

The advent of [1] changes that situation, so @robert-hh and me are striving towards making the Python part blend into each other seamlessly.

In other words, the pure-Python library should be made API compatible with [2].

With kind regards,
Andreas.

[1] pycom/pycom-micropython-sigfox#356
[2] https://github.com/micropython/micropython/tree/master/drivers/onewire

@amotl
Copy link
Contributor Author

amotl commented Nov 17, 2019

For keeping both versions around within a single repository, I propose putting them in different directories instead of calling them xxx_new.py.

Within setup.mk, we called them xxx_python.py vs. xxx_native.py. In order to map this idea with directories, we might just call them python vs. native.

However, we might want to skip the native part altogether when adding additional convenience functionality through an adapter class as outlined within #11.

@amotl
Copy link
Contributor Author

amotl commented Nov 17, 2019

Right now, we've introduced a compatibility adapter called DS18X20NativeDriverAdapter. After getting the lowlevel drivers compatible with each other, we will be able to get rid of that altogether.

@amotl
Copy link
Contributor Author

amotl commented Nov 17, 2019

In order to make both libraries compatible with each other, we should also send a PR to Genuine MicroPython which adds functionality to the convert_temp method to start the conversion on a specific sensor device by obtaining a rom argument like the read_scratch and write_scratch methods.

@amotl
Copy link
Contributor Author

amotl commented Nov 17, 2019

We should also take care not to communicate with the hardware within the object constructor already. This has already bitten the C-driver for the HX711 sensor which we recently refactored not to do this anymore through bogde/HX711#123.

@amotl
Copy link
Contributor Author

amotl commented Nov 17, 2019

We should also take care not to communicate with the hardware within the object constructor already.

I can see this is currently the case through

self.ow.writebyte(self.ow.CMD_SKIPROM)
self.ow.writebyte(CMD_RDPOWER)
self.powermode = self.ow.readbit()

As we got rid of invoking the scan() method from within the constructor, we might think about putting this into a specific method as well. Is it actually about activating the parasite power mode the DS18B20 is capable of?

Elsewhere, this is done through a readPowerSupply method.

@robert-hh
Copy link
Owner

Already done, but not optimal. Because then the mode is only known to the driver if the method has been called. And adding another check in each and every call is nasty too.

@amotl
Copy link
Contributor Author

amotl commented Nov 17, 2019

As the driver API is already compatible with Genuine MicroPython, we might want to close this issue and divert discussions about different topics into another issues.

Thanks a bunch for all your work on this, @robert-hh!

@amotl amotl closed this as completed Nov 17, 2019
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

No branches or pull requests

2 participants