-
-
Notifications
You must be signed in to change notification settings - Fork 654
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
Honor the channel mask while trying to join the network. #45
Honor the channel mask while trying to join the network. #45
Conversation
Can you check your logic on line 886 (left-hand line number column), please? I believe it's inverted from the comments. After applying your patch and watching the channel frequencies as it's trying to join, it's using frequencies outside the enabled sub-band. |
@jcwren [edited] After asking @terrillmoore, the code looks correct as it stands. If last transmit we were not using DR_SF8C, we select a 500kHz channel based on last channel tried and switch to DR_SF8C. Otherwise, the last attempt was DR_SF8C, so go back and try another 125kHz channel. |
@jcwren Not sure what to suggest regarding the channel selection you're seeing. IIRC the subband selection is 0 based, so selecting the second sub-band group requires selectBand(1). |
If that's the case then I would argue that the comments are misleading and do not indicate that the intent is indeed to flip-flop between DR_SF8C and not-DR_SF8C when attempting to join. That being said, why would you ever want to use any channel outside the enabled sub-band(s), regardless of whether you're joining or not? Certainly you can limit the sub-band once you've joined, but if you're operating on a network that you already know is in a certain sub-band, why waste battery power, time, and bandwidth on channels that won't exist? If you agree that it should not, I can provide debug output showing the frequencies it's stepping through as it joins. |
Clearly obeying the active channel map is the right thing to do. That's why Terry made this change in the first place, because the prior code was ignoring the map. I'm not suggesting that the channel selection you observe is a good thing, just that I don't see how this code is responsible. I recommend printing the active channels from the map (just 8 hex bytes) as well as the channel selected right after the |
@jcwren You could also try using The Things New York's fork https://github.com/things-nyc/arduino-lmic/commits/master |
@jcwren If you are seeing weird channel usage during join, please look at issue mcci-catena#1 and the related fixes. There was a definite bug (not at this location in the code) related to the reduction of DR -- totally US915-specific. The code generates bogus indices for an array and therefore does some odd things (and eventually ASSERTs, at least on my platform; but what it does is very much platform dependent, because it is using out-of-range indices on an array). I have noticed that the join behavior improved noticeably with all the fixes I made in my fork, but I have not had time (and don't expect to have time) to identify which fixes improved matters. I have done substantial testing recently, and in my use (Cortex M0 and RISC-V CPUs, all with Hope-RF radios), the channel access pattern is exactly as expected. No use of channels outside the subband either pre- or post-join. The related fixes have been pushed to The Things NYC, but I don't know if Frank has had time to review it. |
Gotta love github, so many forks and branches of code to choose from. Looks like I should switch to at least the The Things NYC branch, if not @terrillmoore's. Is there a compelling reason to pick one over the other? |
My intent with the TTNNYC fork was to hew closely to Matthias' "original" and push our fixes to him. But since he hasn't been pulling, we diverge. @terrillmoore has been producing a commercial product and has spent the most time both building and running hardware and debugging the code. He's less interested in minimizing change, so he may have some extra dependencies or things to support his C++ wrapper. Starting afresh, I'd go with the mcci-catena to benefit from Terry's time. |
My recommendation would be to stay as close to the “center” as you can. I think that “The Things NYC” has the most active US915 users at the moment, so I think that might be a good choice. I’m open-sourcing MCCI’s work, but I have limited time, and my repo is in the middle of several Arduino and non-Arduino projects… so using it might be more difficult.
|
This repository is now deprecated, see #297 for some more background. I'm grateful for your contribution, but it will no longer be merged. I'm recommending people to use the MCCI version of LMIC instead. If this PR addresses an issue that also exists in that version, I would encourage you to resubmit your contribution there, so it might benefit other users. I'm sorry for the inconvenience... |
Also fixes a bug where nextJoinState() chooses unrelated 500kHz channel rather than the one associated with the current channel's sub-band.