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] Add MSoM platform support #2681

Merged
merged 16 commits into from Aug 14, 2023
Merged

Conversation

scott-brust
Copy link
Member

@scott-brust scott-brust commented Aug 7, 2023

Problem

Support for MSoM hardware platform

Solution

Updated to support DVT pinmap, various other fixes

Steps to Test

Example App

References

⚠️ [NOTE]: This PR will introduce breaking changes with TrackerM Serial2/Serial3 definitions. ⚠️


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)

HAL_USART_SERIAL4 = 3,
HAL_USART_SERIAL5 = 4,
HAL_USART_SERIAL1 = 0, // maps to Serial1
HAL_USART_SERIAL2 = 1, // may map to NCP or Serial2, see HAL_PLATFORM_CELLULAR_SERIAL / HAL_PLATFORM_WIFI_SERIAL
Copy link
Contributor

Choose a reason for hiding this comment

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

All of these comments are identical. Is this intended?

Copy link
Member Author

Choose a reason for hiding this comment

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

These were added when we reordered the USART serial definitions, ill let @technobly comment

#define HAL_PLATFORM_POWER_MANAGEMENT_OPTIONAL (1)

#define HAL_PLATFORM_RADIO_ANTENNA_INTERNAL (0)
#define HAL_PLATFORM_RADIO_ANTENNA_EXTERNAL (0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be set to '1'?

Copy link
Member Author

Choose a reason for hiding this comment

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

(I posted this comment previously but it got deleted somehow)

Its a good question. I think HAL_PLATFORM_RADIO_ANTENNA_EXTERNAL is intended for platforms that have both an internal and external wifi / ble antenna ( ie P2, Argon, etc). This define allows selectAntenna() to control the IO needed to switch between an external and internal antenna for BLE/WIFI.
MSoM does not appear to have a switch, so in effect it only has the external BLE/WIFI antenna. So it just always defaults to that and calling
selectAntenna()
will return
SYSTEM_ERROR_NOT_SUPPORTED
So I dont think we need to enable these options.

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as SYSTEM_ERROR_NOT_SUPPORTED is returned and getAntenna() returns the external one then we can pass over this setting.

hal_usart_end(HAL_USART_SERIAL2);
hal_usart_begin_config(HAL_USART_SERIAL2, 57600, SERIAL_8N1, nullptr);
#endif
#if HAL_PLATFORM_CELLULAR
Copy link
Contributor

Choose a reason for hiding this comment

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

This will change later?

Copy link
Member Author

Choose a reason for hiding this comment

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

I dont believe so. HAL_PLATFORM_CELLULAR is intended to pair with HAL_PLATFORM_CELLULAR_SERIAL below. IE if platform cellular is defined, then there should be a serial interface for that cellular defined as well.

hal/src/rtl872x/inet_hal_compat.cpp Outdated Show resolved Hide resolved
hal_usart_end(HAL_USART_SERIAL2);
hal_usart_begin_config(HAL_USART_SERIAL2, 57600, SERIAL_8N1, nullptr);
#endif
#if HAL_PLATFORM_CELLULAR
Copy link
Member Author

Choose a reason for hiding this comment

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

I dont believe so. HAL_PLATFORM_CELLULAR is intended to pair with HAL_PLATFORM_CELLULAR_SERIAL below. IE if platform cellular is defined, then there should be a serial interface for that cellular defined as well.

hal/inc/hal_dynalib_wlan_compat.h Outdated Show resolved Hide resolved
hal/inc/inet_hal_compat.h Outdated Show resolved Hide resolved
hal/src/trackerm/hal_platform_config.h Show resolved Hide resolved
hal/src/tron/hal_platform_config.h Show resolved Hide resolved
@@ -1379,6 +1379,7 @@ int QuectelNcpClient::registerNet() {
CHECK_TRUE(r == 1, SYSTEM_ERROR_UNKNOWN);
r = CHECK_PARSER(respNwMode.readResult());
CHECK_TRUE(r == AtResponse::OK, SYSTEM_ERROR_UNKNOWN);
#if PLATFORM_ID == PLATFORM_MSOM
Copy link
Member

Choose a reason for hiding this comment

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

If we do want to enable 2G fallback on Tracker, we have enough space for 5.5.0. Just not in develop for now.

@scott-brust scott-brust merged commit b7b1b93 into develop Aug 14, 2023
13 checks passed
@scott-brust scott-brust deleted the feature/add-msom-platform branch August 14, 2023 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants