Cellular: retire CellularBase class #9746
Conversation
It was decided to retire CellularBase class which served as a pure virtual interface class from which Cellular network stack implementations would get inherited. However, the current view is that we may be the only user of it so we could retire CellularBase.
|
Rather than refactor, should this be functionality change ? It's backward compatible as we just deprecating but removing one? In any case, this should be covered by in the release notes (add Release notes section to the first comment here please) |
|
As requested release notes section has been added. |
|
@blind-owl, thank you for your changes. |
|
|
||
| /** @copydoc NetworkInterface::cellularBase | ||
| */ | ||
| MBED_DEPRECATED_SINCE("mbed-os-5.12", "Migrated to CellularInterface") |
There was a problem hiding this comment.
How can something new be already deprecated?
Did you copy&paste the code without removing the deprecation notices?
There was a problem hiding this comment.
Ah.. just noticed that this is just to maintain backward compatibility.
| */ | ||
| virtual void set_default_parameters(); | ||
| }; | ||
| MBED_DEPRECATED_SINCE("mbed-os-5.12", "Migrated to CellularInterface") |
There was a problem hiding this comment.
Is this working?
Does it actually cause deprecation notice to come up?
There was a problem hiding this comment.
It was decided to retire CellularBase class which served as a pure
virtual interface class from which Cellular network stack
implementations would get inherited. However, the current view is that
we may be the only user of it so we could retire CellularBase.
Is this sufficient info for this deprecation? No migration needed?
|
This is backwards compatible (CellularBase == CellularInterface), so no real impacts to existing users. |
|
@donatieng / @SenRamakri Please review @AnotherButler Please review the docs |
|
CI started whist waiting on final reviewers |
|
@0xc0170 Remember that @AnotherButler will review the Review Notes once they're consolidated. |
Test run: FAILEDSummary: 1 of 8 test jobs failed Failed test jobs:
|
|
CI restarted |
Test run: SUCCESSSummary: 12 of 12 test jobs passed |
Waiting for the final approval |
There was a problem hiding this comment.
If this is just renaming CellularBase to CellularInterface, I personally approve on naming consistency grounds. It's inline with all the rest of the IP core stuff.
#4119 for Mbed OS 5.5 started with CellularInterface, but it got turned into CellularBase during review. (Mainly feedback from @geky and @sg-, as I recall). IIRC, it was partly confusion about whether it was a "C++ interface" or a "network interface" (it's actually both, of course).
No corresponding changes were ever made anywhere else in the networking stuff, so that's just left it just an odd one out, which is annoying. (And I'm unclear how CellularInterface continued to exist alongside CellularBase)
Description
It was decided to retire CellularBase class which served as a pure
virtual interface class from which Cellular network stack
implementations would get inherited. However, the current view is that
we may be the only user of it so we could retire CellularBase.
Test coverage:
Pull request type
Reviewers
Release Notes
In order to align to other connectivity technologies CellularBase is deprecated and CellularInterface usage is preferred. This change is backward compatible.