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

Default to 2Mbaud for Sonoffs? #101

Open
XenoKovah opened this issue Oct 9, 2024 · 6 comments
Open

Default to 2Mbaud for Sonoffs? #101

XenoKovah opened this issue Oct 9, 2024 · 6 comments

Comments

@XenoKovah
Copy link

If auto-detection for baud is currently supposed to work, then this is a bug report that it doesn't.

But I think it's not supposed to, due to it being mentioned on past tickets that differentiating the slow vs. fast devices is difficult or possibly impossible. But my request would be that we default to assuming that everyone can get the fast version of the chip these days, and that the slow version was just an anomaly from supply chain shortages during the pandemic. So my request is that sniffle_hw.py is updated to default to 2000000 for Sonoffs and let people with the slow version do the override, rather than people with the fast version.

diff --git a/python_cli/sniffle/sniffle_hw.py b/python_cli/sniffle/sniffle_hw.py
index 102b669..4474372 100644
--- a/python_cli/sniffle/sniffle_hw.py
+++ b/python_cli/sniffle/sniffle_hw.py
@@ -115,7 +115,8 @@ class SniffleHW:
                 else:
                     baud = 921600
         elif is_cp2102(serport):
-            baud = 921600
+#            baud = 921600
+            baud = 2000000

I say this just because it'd be nice if I didn't have to override the baud at all and could just have students use no-diffs Sniffle in our upcoming OST2 BLE class...

@sultanqasim
Copy link
Collaborator

I was contemplating doing the same. I defaulted to 921600 because as you say, I couldn't find a reliable way to distinguish between CP2102 and CP2102-based devices, so just encouraged everyone to use 921600 baud on CP2102 devices since it's generally "fast enough". However, adding a baud rate option and defaulting to 2M baud for all devices may be a better option going forward, given that the CP2102 (non-N) Sonoff dongles seem to have been a ~2022 era anomaly.

@XenoKovah
Copy link
Author

Random question: would this change also make the auto-detection simpler? (And thus work by default again for Sonoff /dev/ttyUSB* devices.) Because I just remembered that I think with the TI dev boards I didn't need to always be passing the -s option, it just auto-detected compatible devices, but with Sonoffs I seem to always need to pass -s. If so, that seems like another good reason to do it.

@sultanqasim
Copy link
Collaborator

It should already be auto detecting Sonoff dongles when no -s option is specified (and use the 921600 baud rate with them). See this function:

def find_sonoff_serport():

Is there something different about your Sonoff dongles that prevents it from auto detecting them?

@XenoKovah
Copy link
Author

XenoKovah commented Oct 13, 2024

Is there something different about your Sonoff dongles that prevents it from auto detecting them?

Looks like it's down to where I do the baud edit for 2Mbaud I guess:

    def __init__(self, serport=None, logger=None, timeout=None):
        baud = 2000000
        if serport is None:
            serport = find_xds110_serport()
            if serport is None:
                serport = find_sonoff_serport()
                if serport is None:
                    serport = find_catsniffer_v3_serport()
                    if serport is None:
                        raise IOError("Sniffle device not found")
                else:
#                    baud = 921600
                    baud = 2000000 # This makes auto-detection work
        elif is_cp2102(serport):
#            baud = 921600
            baud = 2000000 # I only usually change this

So in this sense, yes defaulting back to 2000000 instead of 921600 would make Sonoffs "just work" for me for autodetection too.

@mdxs
Copy link
Contributor

mdxs commented Oct 17, 2024

I think that PR #102 actually proposes (amongst other changes) to add an explicit --baudrate command line argument, not sure how this exactly fits this discussion as I don't use/need it myself... but maybe something to look at in this context.

@XenoKovah
Copy link
Author

XenoKovah commented Oct 17, 2024

I agree that the --baudrate option would seem to achieve my request, as long as it's paired with the removal of any straggler references to 921600 like this: https://github.com/nccgroup/Sniffle/pull/102/files#r1805402099

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

3 participants