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

[rtl872x] Implement WiFi.selectAntenna #2651

Merged
merged 4 commits into from
Apr 20, 2023

Conversation

scott-brust
Copy link
Member

Problem

WiFi.selectAntenna() is not implemented for P2

Solution

On P2, BLE and WiFi share the same physical antenna, so use selectAntenna() from radio_common.cpp

Steps to Test

Use test app below to verify antenna switches

Example App

#include "Particle.h"

SYSTEM_MODE(AUTOMATIC);
SYSTEM_THREAD(ENABLED);

SerialLogHandler logHandler(LOG_LEVEL_ERROR,
{
    { "app", LOG_LEVEL_ALL },
});

String anntena_names[] = {"Internal", "External", "", "Automatic"};

void setup() {
    
}
 
void loop() {
    if (!Serial.available()) {
        return;
    }

    while (Serial.available()) {
        char c = Serial.read();
        Log.info("Got %c", c);
        int result = 0;
        if (c == 'i') {
            result = WiFi.selectAntenna(ANT_INTERNAL);
        }
        else if (c == 'e') {
            result = WiFi.selectAntenna(ANT_EXTERNAL);
        }
        else if (c == 'a') {
            result = WiFi.selectAntenna(ANT_AUTO);
        }
        else if (c == 'n') {
            result = WiFi.selectAntenna(ANT_NONE);
        }

        String antenna_name = "";
        WLanSelectAntenna_TypeDef antenna = WiFi.getAntenna();
        if (antenna <= ANT_AUTO) {
            antenna_name = anntena_names[(int)antenna];
        }

        Log.info("WiFi.selectAntenna result: %d now using %s antenna", result, antenna_name.c_str());

        if (result == SYSTEM_ERROR_NONE) {
            WiFiAccessPoint aps[20];
            int found = WiFi.scan(aps, 20);
            for (int i=0; i<found; i++) {
                WiFiAccessPoint& ap = aps[i];
                Log.info("ssid=%s security=%d channel=%d rssi=%d", ap.ssid, (int)ap.security, (int)ap.channel, ap.rssi);
            }    
        }
    }
}

Test output with no external antenna

0000012710 [app] INFO: Got n
0000012717 [app] INFO: WiFi.selectAntenna result: -120 now using Automatic antenna
0000017798 [app] INFO: Got i
0000017968 [app] INFO: WiFi.selectAntenna result: 0 now using Internal antenna
0000022089 [app] INFO: ssid=derp security=3 channel=9 rssi=-47
0000022105 [app] INFO: ssid=derp5 security=3 channel=153 rssi=-55
0000025071 [app] INFO: Got e
0000025238 [app] INFO: WiFi.selectAntenna result: 0 now using External antenna
0000029357 [app] INFO: ssid=derp5 security=3 channel=153 rssi=-70
0000029374 [app] INFO: ssid=derp security=3 channel=9 rssi=-88
0000033136 [app] INFO: Got a
0000033307 [app] INFO: WiFi.selectAntenna result: 0 now using Automatic antenna
0000037429 [app] INFO: ssid=derp security=3 channel=9 rssi=-47
0000037444 [app] INFO: ssid=derp5 security=3 channel=153 rssi=-55

Switching from Auto / Internal to external shows a drop in RSSI


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)

@scott-brust scott-brust marked this pull request as ready for review April 18, 2023 23:38
hal/network/ncp/wifi/wlan_hal.cpp Outdated Show resolved Hide resolved
}

WLanSelectAntenna_TypeDef wlan_get_antenna(void* reserved) {
return ANT_AUTO;
return current_antenna;
Copy link
Member

Choose a reason for hiding this comment

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

The setting is already saved in DCT (DCT_RADIO_ANTENNA_OFFSET). Just needs to be exposed in radio_common stuff.

}
uint8_t dctAntenna = 0xff;
CHECK(dct_read_app_data_copy(DCT_RADIO_ANTENNA_OFFSET, &dctAntenna, sizeof(dctAntenna)));
if ((radio_antenna_type)dctAntenna <= RADIO_ANT_EXTERNAL) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Should we validate the DCT data or just return it if its present?

* @param antenna Currently configured Antenna type.
* @return 0 on success or a negative result code in case of an error.
*/
int getRadioAntenna(radio_antenna_type * antenna);
Copy link
Member

Choose a reason for hiding this comment

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

Nit/style: radio_antenna_type* antenna, same above

return SYSTEM_ERROR_INVALID_ARGUMENT;
}
uint8_t dctAntenna = 0xff;
CHECK(dct_read_app_data_copy(DCT_RADIO_ANTENNA_OFFSET, &dctAntenna, sizeof(dctAntenna)));
Copy link
Member

Choose a reason for hiding this comment

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

I'd do exactly the same thing as currently done in initRadioAntenna() and return default normally unless dct read value is exactly INTERNAL or EXTERNAL and replace that block with getRadioAntenna() so that we don't have duplicate logic.

@scott-brust scott-brust merged commit 2700304 into develop Apr 20, 2023
@scott-brust scott-brust deleted the fix/sc-117838/wifi_select_antenna branch April 20, 2023 17:12
@scott-brust scott-brust added this to the 5.3.2 milestone May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants