-
Notifications
You must be signed in to change notification settings - Fork 3k
Ublox drivers #2977
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
Ublox drivers #2977
Conversation
Master shall be fine but not certain if release version for 5 is valid in this case (the requirement gcc + iar + armcc support). Notes:
|
02088ca
to
fdf4e94
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All your headers have license that we can't really accept, could you update it to PBL please.
Also you're using global state, which for now is fine, but when we change the implementation to support multiple stacks could possibly be an issue.
public: | ||
/** OdinWiFiInterface lifetime | ||
*/ | ||
OdinWiFiInterface(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation is all over the place in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks will fix!
@@ -0,0 +1,193 @@ | |||
/*--------------------------------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can get that 'license' in. You should license your headers the same way you license your binary that is PBL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, we will look into if it's possible to use this one instead:
https://www.mbed.com/en/licenses/permissive-binary-license/
@@ -1186,7 +1186,7 @@ | |||
"extra_labels": ["STM", "STM32F4", "STM32F439", "STM32F439ZI"], | |||
"macros": ["HSE_VALUE=24000000", "HSE_STARTUP_TIMEOUT=5000", "CB_INTERFACE_SDIO","CB_CHIP_WL18XX","SUPPORT_80211D_ALWAYS","WLAN_ENABLED"], | |||
"inherits": ["Target"], | |||
"device_has": ["ANALOGIN", "CAN", "I2C", "I2CSLAVE", "INTERRUPTIN", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "SERIAL", "SLEEP", "SPI", "SPISLAVE", "STDIO_MESSAGES"], | |||
"device_has": ["ANALOGIN", "EMAC", "CAN", "I2C", "I2CSLAVE", "INTERRUPTIN", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "SERIAL", "SLEEP", "SPI", "SPISLAVE", "STDIO_MESSAGES"], | |||
"features": ["LWIP"], | |||
"release_versions": ["5"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets remove the release version until we get this cleaned up, formatting and three compilers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sam do you think it's allright to keep the release version as we cleaned stuff up and got all 3 compilers in place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there isnt support for the 3 compilers this will fail CI and should not be turned on until it passes all CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is support for all 3 compilers now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it passes CI, then we're good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see - I'll start the tests.
@@ -1186,7 +1186,7 @@ | |||
"extra_labels": ["STM", "STM32F4", "STM32F439", "STM32F439ZI"], | |||
"macros": ["HSE_VALUE=24000000", "HSE_STARTUP_TIMEOUT=5000", "CB_INTERFACE_SDIO","CB_CHIP_WL18XX","SUPPORT_80211D_ALWAYS","WLAN_ENABLED"], | |||
"inherits": ["Target"], | |||
"device_has": ["ANALOGIN", "CAN", "I2C", "I2CSLAVE", "INTERRUPTIN", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "SERIAL", "SLEEP", "SPI", "SPISLAVE", "STDIO_MESSAGES"], | |||
"device_has": ["ANALOGIN", "EMAC", "CAN", "I2C", "I2CSLAVE", "INTERRUPTIN", "PORTIN", "PORTINOUT", "PORTOUT", "PWMOUT", "SERIAL", "SLEEP", "SPI", "SPISLAVE", "STDIO_MESSAGES"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see where EMAC is used in this patch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Macro is used on the mbed-os side, to pull in right bits for the EMAC API.
fdf4e94
to
ec9113a
Compare
4c3089b
to
62bc686
Compare
/morph test |
Ah actually that was pointless as there is no Odin boards in the farm... @andreaslarssonublox can you run the test locally and paste the results here please. |
@sg- we seem to have Ethernet tests failing, that's expected as we can't have both Ethernet and WiFi working at the same time. So we can either by default disable WiFi (EMAC flag) or change tests not to run in case of EMAC. We should be able to fix it properly when we get the network unification done. |
There were problems in the wifi_emac_api.cpp that have been fixed now when removing EMAC in target.json. I will push the file once I got some test results. |
I ran one test case for eth/lwip without EMAC in target.json to verify that it works: Will rerun the test cases for the standard configuration(with EMAC) for the new commit for all compilers. Would that be enough? |
All OK except lwip/eth as expected for GCC/ARM/IAR toolchain: |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
Outputmbed Build Number: 0 All builds and test passed! |
Currently only one network interface can be compiled in at a given time, to avoid failing automated test on this target WiFi will be disabled by default.
Added commit that removes "EMAC" from targets.json effectively disabling WiFi by default to avoid ethernet failures in automated tests. |
/morph test |
@bulislaw Thanks for that, this next test is just to ensure the build is ok. /morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
Outputmbed Build Number: 0 All builds and test passed! |
Please ignore the X on the morph-test status, I accidentally triggered another build and canceled it. Test results LGTM 👍 |
Description
This is the first binary for the u-blox ODIN-W2 wireless drivers.
Don't know whether this should go into a branch or master.
Limitations
Status
IN DEVELOPMENT
Migrations
NO
Related PRs
#2874
Todos
A couple of Wi-Fi test cases should be written but this is outside of this PR.
The existing lwip tests fail now so they shall be adapted or excluded.
For u-blox mbed 3 ODIN-W2 driver C API there is some documentation here:
https://github.com/u-blox/ublox-odin-w2-drivers-binary
@screamerbg
Should some similar documentation for mbed 5 go into the sdk-folder or is it better suited on u-blox homepage or the board page hosted by ARM?
Deploy notes
Steps to test or reproduce
The binary was built based on the following in mbed-os:
SHA-1: 62bc686
The binary was built based on the following u-blox ODIN-W2 drivers:
SHA-1: 251f79b706178e97eb1445dba2e17304ec782a14
Some basic tests have been manually done:
connect open, connect WPA2, scan, get RSSI, disconnect.