fix(s7commplus): remove misleading TLS hint

[gijzelaerr]

Summary

• stop treating a non-zero CreateObject return value as evidence that TLS is required
• share the same status logging between synchronous and asynchronous clients
• clarify that parsing continues because affected PLCs can still return usable session data

Testing

• uv run --no-sync pytest (1823 passed, 78 skipped)
• uv run --no-sync pre-commit run --all-files

Progresses #710.

[gijzelaerr]

@bvanelli I requested your review because this change is directly based on the CreateObject behavior seen in the recent PLC traces: a non-zero return value is logged without falsely claiming that TLS is required, while usable session parsing continues.

Could you check that the revised interpretation and sync/async behavior match what you observed? No additional byte capture is needed for this one.

[bvanelli]

@bvanelli I requested your review because this change is directly based on the CreateObject behavior seen in the recent PLC traces: a non-zero return value is logged without falsely claiming that TLS is required, while usable session parsing continues.

Could you check that the revised interpretation and sync/async behavior match what you observed? No additional byte capture is needed for this one.

I assume you are talking about my change (Int16)ReturnValue < 0 for the Legitimation?

I did check the reference implementation, and those seems like fundamentally different things. Also, S7CommPlusConnection.Connect() does not seem to check it at all, and it seems to go straight to TLS every time:

https://github.com/thomas-v2/S7CommPlusDriver/blob/dbd61e447c7aaf4486cf1f1fe0201212a6bd93c8/src/S7CommPlusDriver/S7CommPlusConnection.cs#L436-L445

Is there a historical reason why this warning (or the flag tls=True) exists?

[gijzelaerr]

The historical reason is #760: when the CreateObject return value was first surfaced, I inferred from that TLS failure that a non-zero value generally meant the session was being rejected and that TLS was the likely remedy. That was an incident-driven assumption, not a rule established by a capture or the reference implementation. The newer traces show that inference is too strong because usable session data can follow a non-zero value. You are also right that the reference connection path goes directly to TLS; our client additionally retains V1 non-TLS support. This PR only removes the TLS claim from CreateObject status logging. It does not change the separate signed ReturnValue check used during legitimation.

[gijzelaerr]

@bvanelli Following up now that I answered the background question: could you take another look at the current wording and approve if the rationale is clear?

[bvanelli]

Looks good, my understanding is that this error should never appear unless there is a programming error from the protocol.

Just two questions:

• The log message was changed from f-string to interpolation. You (or I) can follow and enforce that via ruff. It is probably worth it for consistency repo-wide.
• Is the final goal to just get rid of the tls flag? Because if you use S7commplus, you are probably aware of the upgrade (or want it to be automatic).

[gijzelaerr]

Thanks for the review. The logging interpolation change is deliberate: it avoids eagerly formatting messages that may be filtered out, and it matches the style recommended by Python logging. Enforcing that consistently through Ruff could be useful, but I would keep the repo-wide mechanical cleanup separate from this focused behavior fix.

There is no current plan to remove use_tls. The client supports both legacy V1 SessionKey connections without TLS and newer TLS-based paths, and a failed TLS ClientHello can cause some PLCs to reset the connection. That makes unconditional TLS or speculative in-place negotiation unsafe. Automatic selection may be worth adding once it can reconnect and choose from reliable device/protocol evidence, but this PR only removes the unsupported claim that this particular CreateObject return value means TLS is required.