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

Improve startup sequence to be compliant with the spec #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

emontnemery
Copy link

Improve startup sequence to be compliant with the spec:

  • The initial request message shall be sent at 300 baud
  • There should not be any delay between the NUL characters in the wakeup message for battery powered devices, and since the wakeup message is sent at 300 baud we can send a predetermined number of NUL characters

@Krolken
Copy link
Contributor

Krolken commented Aug 18, 2021

Hi. thanks for the PR.

What problem are you intending to solve with this PR?
It seems like we both have misread the standard. I double checked now and it is 5 ms delay between the null characters.

I don't think the baudrate bound solution is the best since this is not reliable when sending over TCP, which is what I do mostly in production. But it could be updated to reflect timings in the standard a bit better. I have never had problems waking a device up though.

        BATTERY_STARTUP_SEQUENCE_TIME = 2.2
        INTER_NULL_CHAR_TIMEOUT = 0.0025
        POST_BATTERY_STARTUP_WAIT_TIME = 1.5
        duration = 0
        start_time = time.time()
        logger.info("Sending battery startup sequence")
        while duration < BATTERY_STARTUP_SEQUENCE_TME:
            out = b"\x00"
            self.transport.send(out)
            time.sleep(INTER_NULL_CHAR_TIMEOUT) 
            duration = time.time() - start_time
        logger.info("Startup Sequence finished")
        self.rest(POST_BATTERY_STARTUP_WAIT_TIME)

The _current_baudrate is completly useless and should be removed. But to force the startup seqeuence to 300 baud will be troublesome since some meters are not following standard and have fixed baud rates on their interfaces.

I have an idea to rewrite the lib with a sans-io approach to that suff like this is more clearly handled.

@emontnemery
Copy link
Author

emontnemery commented Aug 18, 2021

It seems like we both have misread the standard. I double checked now and it is 5 ms delay between the null characters.

No, that's not the case, the standard specifies a maximum delay of 5 ms:
Between two NUL characters of this message a maximum delay time of 5 ms is allowed.

The problem I'm trying to solve is to precisely make the wakeup work reliably over network, my meter failed to wake up reliably with the delays; maybe it honors the 5 ms maximum delay time and other meters accept delays which are orders of magnitude out of spec?

In your case:

I don't think the baudrate bound solution is the best since this is not reliable when sending over TCP, which is what I do mostly in production.

On the other end of the network, there must be a serial port with a fixed or otherwise known baud rate?
Also, there's no synchronization method in the TcpTransport, TcpTransport._send just writes to the socket.

to force the startup seqeuence to 300 baud will be troublesome since some meters are not following standard and have fixed baud rates on their interfaces.

I see, and my meter does not allow initiating communication at other baudrates than 300.
Should the initial baudrate be an option to the client's constructor?

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.

2 participants