paller / wavin-controller

A controller for Wavin AHC 9000 written in Python with focus for use in home-automation.
11 stars 4 forks source link

Make the serial communication more robust #5

Closed paller closed 5 years ago

paller commented 6 years ago

Add the possibility for setting a default retry count for all serial communication. This is useful in the case when there's just too much noise and the communication often fails and the user really don't care to handle the raised exceptions.

At the same time add some logging such that it's possible to get a visual of the failure rate.

lauer commented 5 years ago

I do hit this timeout when I want to read multiple sensors in one loop. I have also been looking at this project. https://github.com/spiff42/wavin-python

He is using minimalmodbus, which seems more simple to use, and does not timeout even when reading a lot of values.

Could one solution be to switch from serial to this library?

paller commented 5 years ago

I looked at various modules when starting this project and for reasons I can't recall I chose not to use them.

Some of the reasons I had not to use a module:

Add to that the strange way Wavin is using modbus it just seemed easier to make it from scratch. In hindsight I can say it really helped with the understanding of their documentation.

Anyway, back to your request. At first I didn't think it would be necessary with more robust communication as any application could handle that by itself. I later changed my mind and made this issue and it seems about time I get around to it.

I was thinking of making a dumb retry-loop but you say that spiff42/minimalmodbus doesn't have this problem and if that's the case when running on the same hardware it would seem I have a bug somewhere. Even though my own failure rate was rater high for such a short wiring and slow baudrate I was blaming it on transmission noise. Looking at the minimalmodbus implementation there might just be something I'm missing as they have some sort of timed delay between reading and writing on the bus.

From minimalmodbus function _communicate():

   if sys.version_info[0] > 2:
        request = bytes(request, encoding='latin1')  # Convert types to make it Python3 compatible

    # Sleep to make sure 3.5 character times have passed
    minimum_silent_period   = _calculate_minimum_silent_period(self.serial.baudrate)
    time_since_read         = time.time() - _LATEST_READ_TIMES.get(self.serial.port, 0)

    if time_since_read < minimum_silent_period:
        sleep_time = minimum_silent_period - time_since_read
        time.sleep(sleep_time)

    # Write request
    latest_write_time = time.time()

    self.serial.write(request)

I would like to test something similar before making the dumb solution.

Would you be interested in testing it on your setup?

lauer commented 5 years ago

Yes, I would be happy to test. I am sitting right now and try learn some python, to let me create a script which add sensor stats to my monitoring platform every 5 minut.

paller commented 5 years ago

After some trial and error I finally nailed it. Kinda embarrassing actually.

The missing silence delay was a complete dead end. As I'm using a FTDI chip and they generally know what they are doing I would expect the hardware to control that part. Anyway, I tried it and it didn't make any difference whatsoever.

Instead, switching from one to two stop-bits made all my failed readings disappear. I'm now getting a valid response on all read requests. You can check it out on the dev branch commit 58be306a.

Please let me know if it doesn't work.

lauer commented 5 years ago

I have just testet it, and after a timeout on first run, where the 12 sensor-registers was pulled and the second failed, I tried to switch to STOPBITS_TWO - and it worked.

Currently I am fetching data every 10 minute, and logging if any errors occur.

lauer commented 5 years ago

I guess you can close this, I had no error since TOPBITS_TWO

paller commented 5 years ago

Fixed by 7bc2a813