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.

Sign up for GitHub

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

Jump to bottom

[Cellular/Wiring] Remove rssi and qual from Cellular.RSSI() #2212

Merged
merged 6 commits into from
Nov 11, 2020
Merged

[Cellular/Wiring] Remove rssi and qual from Cellular.RSSI() #2212

merged 6 commits into from
Nov 11, 2020

Conversation

keeramis
Copy link
Contributor

@keeramis keeramis commented Oct 8, 2020 •
edited
Loading

Description

Remove rssi and qual from Cellular.RSSI()

Additionally,

  • Printing Cellular.RSSI() will now print signalvalue,qualityvalue
  • New API call - Cellular.getSignal() as an alias for Cellular.RSSI().

Steps to Test

Example App

An app checking for Cellular.RSSI().rssi or Cellular.RSSI().qual should throw errors.

void setup() {
}

void loop() {
}

References

ch62644

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
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@keeramis keeramis added this to the 3.0.0 milestone Oct 8, 2020
sergeuz
sergeuz requested changes Oct 9, 2020
View reviewed changes
hal/inc/cellular_hal.h Outdated Show resolved Hide resolved
hal/inc/net_hal.h Outdated Show resolved Hide resolved
@keeramis keeramis requested a review from sergeuz October 15, 2020 20:40
@keeramis keeramis force-pushed the ch62644/remove_celluluar_rssi branch from 1561d6f to 2ec804d Compare October 16, 2020 00:01
@keeramis keeramis force-pushed the ch62644/remove_celluluar_rssi branch from a266193 to 666ea81 Compare November 5, 2020 01:23
@keeramis keeramis force-pushed the ch62644/remove_celluluar_rssi branch from 666ea81 to 0964350 Compare November 5, 2020 20:55
@keeramis keeramis requested a review from sergeuz November 5, 2020 21:26
sergeuz
sergeuz requested changes Nov 6, 2020
View reviewed changes
wiring/inc/spark_wiring_cellular_printable.h Outdated Show resolved Hide resolved
wiring/inc/spark_wiring_cellular_printable.h Show resolved Hide resolved
wiring/src/spark_wiring_cellular_printable.cpp Outdated Show resolved Hide resolved
wiring/src/spark_wiring_cellular_printable.cpp Outdated Show resolved Hide resolved
wiring/src/spark_wiring_print.cpp Outdated Show resolved Hide resolved
wiring/src/spark_wiring_cellular.cpp Show resolved Hide resolved
@keeramis keeramis requested a review from sergeuz November 6, 2020 19:53
sergeuz
sergeuz approved these changes Nov 9, 2020
View reviewed changes
wiring/src/spark_wiring_cellular.cpp Outdated Show resolved Hide resolved
wiring/src/spark_wiring_cellular_printable.cpp Outdated Show resolved Hide resolved
avtolstoy
avtolstoy approved these changes Nov 11, 2020
View reviewed changes
Copy link
Member

@avtolstoy avtolstoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks okay. If I had to make a minor comment is that https://github.com/particle-iot/device-os/pull/2212/files#diff-e3cf88d4bb00cc447ee595431d5ba61076b397be131a25a1f0cb63747d4702d9R94 should probably be hidden somewhere within a HAL

@keeramis keeramis merged commit af899b6 into develop Nov 11, 2020
@keeramis keeramis deleted the ch62644/remove_celluluar_rssi branch November 11, 2020 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@avtolstoy avtolstoy avtolstoy approved these changes

@technobly technobly technobly approved these changes

@sergeuz sergeuz sergeuz approved these changes

Assignees
No one assigned
Labels
enhancement needs review
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@keeramis @avtolstoy @sergeuz @technobly