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

fixes RSSI regression on G350 (2G) devices [ch35194] #1841

Merged
merged 1 commit into from Jul 9, 2019

Conversation

@technobly
Copy link
Member

commented Jul 2, 2019

This PR also needs to be cherry-picked to release/v1.2.1 for the 1.2.1 default release.

Problem

A regression in the Cellular.RSSI() API was introduced in Device OS 1.2.0-rc.1 PR #1779 where the G350 RSSI functionality was broken.

Solution

Forces ACT_GSM for G350 in functions that query AT+COPS?

Steps to Test

  • Run new test CELLULAR_07_rssi_is_valid in TEST=wiring/no_fixture
  • Try pressing the MODE button on G350 one time to see Signal Strength indicated in "Bars" flashed with a green LED. With 1.2.0-rc.1 / 1.2.1-rc.3 / 1.3.0-rc.1, the G350 will NOT flash 1-5 blips of the green LED. With this PR it should at least flash 1 green blip, if not more.

References

https://community.particle.io/t/no-signal-strength-information-for-electron-2g-on-device-os-1-0-0/47460/16

Known Issues

With the added test CELLULAR_07_rssi_is_valid, it became clear that on Gen 3 devices Boron & B SoM, the Cellular.RSSI() API was not implemented correctly and needs to be fixed. This will be fixed in a future PR to reduce risk to this fix which will make its way into 1.2.1 default without a 1.2.1-rc.4.


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • [N/A] Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

  • [bugfix] fixes RSSI regression on G350 (2G) devices #1841

@technobly technobly added this to the 1.3.0-rc.2 milestone Jul 2, 2019

@technobly technobly requested a review from avtolstoy Jul 2, 2019

@avtolstoy
Copy link
Member

left a comment

I don't have a G350 Electron at hand, so cannot test, but the changes look good.

@technobly technobly changed the title fixes RSSI regression on G350 (2G) devices fixes RSSI regression on G350 (2G) devices [ch35194] Jul 2, 2019

@technobly

This comment has been minimized.

Copy link
Member Author

commented Jul 2, 2019

Thanks @avtolstoy ! I have tested on G350 / U260 / R410 and Boron (actually it fails on Boron though as noted above). I've also created and shared some binaries in the community for the original reporter of this issue to test, so I'm hoping to get more feedback there.

@avtolstoy

This comment has been minimized.

Copy link
Member

commented Jul 2, 2019

actually it fails on Boron though as noted above

Do you have any info about that?

One suggestion I would make is to possibly change the test to not use rssi and qual directly and instead use the new APIs: getAccessTechnology(), getStrength(), getStrengthValue(), getQuality(), getQualityValue().

@technobly

This comment has been minimized.

Copy link
Member Author

commented Jul 9, 2019

Do you have any info about that?

The issue with Boron is that CellularSignal .rssi and .qual are not set to proper values (which seem to be available via the newer vitals calculations).

One suggestion I would make is to possibly change the test to not use rssi and qual directly and instead use the new APIs: getAccessTechnology(), getStrength(), getStrengthValue(), getQuality(), getQualityValue().

@avtolstoy The purpose of the test is to insure the Cellular.RSSI() API is functioning properly for .rssi (and adding .qual wouldn't be a bad idea either), but if we don't have a test yet for the other new vitals functions those could be good to perform a quick smoke test on as well!

@technobly technobly merged commit 645dabd into develop Jul 9, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@technobly technobly deleted the fix/g350-rssi branch Jul 9, 2019

technobly added a commit that referenced this pull request Jul 9, 2019
Merge pull request #1848 from particle-iot/fix/g350-rssi-121
fixes RSSI regression on G350 (2G) devices [ch35194] (original PR #1841)

@technobly technobly modified the milestones: 1.3.0-rc.2, 1.3.1-rc.1 Aug 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.