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: Gemalto Cinterion support for ELS61 and BGS2 #7677

Merged

0xc0170 merged 4 commits into ARMmbed:master from AriParkkila:cell-gemalto

Aug 29, 2018

AriParkkila commented Aug 2, 2018 •

edited

Loading

Description

AT command support for the Gemalto Cinterion cellular modules:

LWIP supported in PPP mode
UDP sockets supported in AT mode

Depends on PR #7860.

Pull request type

[ ] Fix
[ ] Refactor
[ ] New target
[X] Feature
[ ] Breaking change

0xc0170 requested a review from a team

August 2, 2018 09:56

0xc0170 added the needs: review label

AriParkkila force-pushed the cell-gemalto branch from 929c98c to b644d94 Compare

August 3, 2018 08:57

Author

AriParkkila commented Aug 13, 2018

@jarvte @mirelachirica @TeemuKultala please review

jarvte suggested changes

View reviewed changes

features/cellular/framework/targets/GEMALTO/CINTERION/GEMALTO_CINTERION_CellularNetwork.cpp Outdated

    
                  return (reg_type == C_REG || reg_type == C_GREG || reg_type == C_EREG);

              }

              const char *GEMALTO_CINTERION_CellularNetwork::get_apn() const

Contributor

jarvte Aug 13, 2018

no need for this method as _apn is protected in inherited class

features/cellular/framework/targets/GEMALTO/CINTERION/GEMALTO_CINTERION_CellularStack.cpp Outdated

    
                  }

                  socket->created = true;

                  tr_info("Socket %d created (err %d)", socket->id, _at.get_last_error());

Contributor

jarvte Aug 13, 2018

Could be debug so that log is not polluted.

features/cellular/framework/targets/GEMALTO/CINTERION/GEMALTO_CINTERION_CellularStack.cpp Outdated

    
              nsapi_size_or_error_t GEMALTO_CINTERION_CellularStack::socket_sendto_impl(CellularSocket *socket,

                                                                                        const SocketAddress &address, const void *data, nsapi_size_t size)

              {

                  tr_info("Socket %d, sendto %s, len %d", socket->id, address.get_ip_address(), size);

Contributor

jarvte Aug 13, 2018

Could be debug so that log is not polluted.

features/cellular/framework/targets/GEMALTO/CINTERION/GEMALTO_CINTERION_CellularStack.cpp Outdated

    
                  _at.write_bytes((uint8_t *)data, accept_len);

                  _at.resp_stop();

                  tr_info("Socket %d sendto %s, %d bytes (err %d)", socket->id, address.get_ip_address(), accept_len, _at.get_last_error());

Contributor

jarvte Aug 13, 2018

Could be debug so that log is not polluted.

features/cellular/framework/targets/GEMALTO/CINTERION/GEMALTO_CINTERION_CellularStack.cpp Outdated

    
              nsapi_size_or_error_t GEMALTO_CINTERION_CellularStack::socket_recvfrom_impl(CellularSocket *socket, SocketAddress *address,

                                                                                          void *buffer, nsapi_size_t size)

              {

                  tr_info("Socket %d recvfrom %d bytes", socket->id, size);

Contributor

jarvte Aug 13, 2018

Could be debug so that log is not polluted.

features/cellular/framework/targets/GEMALTO/CINTERION/GEMALTO_CINTERION_CellularStack.cpp Outdated

    
                  _at.resp_stop();

                  tr_info("Socket %d, recvfrom %s, %d bytes (err %d)", socket->id, ip_address, len, _at.get_last_error());

Contributor

jarvte Aug 13, 2018

Could be debug so that log is not polluted.

AriParkkila force-pushed the cell-gemalto branch from b644d94 to adae5a1 Compare

August 14, 2018 08:21

Author

AriParkkila commented Aug 14, 2018

Removed redundant get_apn() and fixed tracing level to tr_debug. Also rebased with mbed-os/master.

jarvte approved these changes

View reviewed changes

AriParkkila force-pushed the cell-gemalto branch from adae5a1 to a42d74f Compare

August 17, 2018 07:35

AriParkkila changed the title ~~Cellular: Gemalto Cinterion support for ELS61~~ Cellular: Gemalto Cinterion support for ELS61 and BGS2

mirelachirica approved these changes

View reviewed changes

cmonr added needs: CI needs: preceding PR and removed needs: review needs: preceding PR labels

Contributor

cmonr commented Aug 22, 2018

/morph build

cmonr added the release-version: 5.10.0-rc1 label

mbed-ci commented Aug 22, 2018

Build : FAILURE

Build number : 2871
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7677/

Contributor

0xc0170 commented Aug 23, 2018

Restarting the build, should be fine now (previous failure was known yesterday and was fixed)

/morph build

mbed-ci commented Aug 23, 2018

Build : SUCCESS

Build number : 2881
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7677/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

mbed-ci commented Aug 23, 2018

Exporter Build : SUCCESS

Build number : 2502
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7677/

Contributor

cmonr commented Aug 23, 2018

Pausing Test CI until 5.9.6 PR is merged.
Will restart CI shortly after.

cmonr added the risk: G label

Contributor

cmonr commented Aug 24, 2018

/morph test

Contributor

cmonr commented Aug 24, 2018

/morph uvisor-test

1 similar comment

Contributor

0xc0170 commented Aug 24, 2018

/morph uvisor-test

Contributor

0xc0170 commented Aug 24, 2018

uvisor CI reports this error : GEMALTO_CINTERION_CellularStack.cpp@461,28: 'strtol' is not a member of 'std' please review

0xc0170 added needs: work and removed needs: CI labels

mbed-ci commented Aug 24, 2018

Test : SUCCESS

Build number : 2636
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7677/2636

0xc0170 removed the ready for merge label

Author

AriParkkila commented Aug 27, 2018

@0xc0170 added more comments and removed out-commented code. Also fixed network registration as that has changed in PR #7860, which must be merged first.

cmonr added needs: preceding PR risk: A and removed risk: G labels

Contributor

cmonr commented Aug 27, 2018

@AriParkkila Were a couple of lines missed?

Normally GitHub will collapse review comments when the line disappears from the review.

Author

AriParkkila commented Aug 28, 2018

@cmonr for me, GitHub is a bit too clever in how it handles review comments, do you see something missing there... I'm guessing that due to this PR does not compile without PR #7860 that might mess up how GitHub shows review comments?

AriParkkila force-pushed the cell-gemalto branch from 620656a to 3c8acea Compare

August 28, 2018 06:23

Ari Parkkila added 4 commits

August 28, 2018 02:14


          Cellular: Gemalto Cinterion support for ELS61

761e01f


          Cellular: Added support for the Gemalto/BGS2 cellular module

a14ac31


          Cellular: Added support for the Gemalto/EMS31 cellular module

9d0e3d8


          Cellular: Fixed network registration on Gemalto AT drivers

90fe9de

AriParkkila force-pushed the cell-gemalto branch from 3c8acea to 90fe9de Compare

August 28, 2018 09:20

Author

AriParkkila commented Aug 28, 2018

Please trigger morph builds to see that this passes all tests.

0xc0170 removed needs: preceding PR needs: work labels

0xc0170 approved these changes

View reviewed changes

0xc0170 added the needs: CI label

Contributor

cmonr commented Aug 28, 2018

/morph build

cmonr added risk: G and removed risk: A labels

mbed-ci commented Aug 28, 2018

Build : SUCCESS

Build number : 2943
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7677/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

mbed-ci commented Aug 28, 2018

Exporter Build : SUCCESS

Build number : 2560
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/7677/

mbed-ci commented Aug 29, 2018

Test : SUCCESS

Build number : 2692
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/7677/2692

0xc0170 added ready for merge and removed needs: CI labels

0xc0170 merged commit 70439dd into ARMmbed:master

0xc0170 removed ready for merge risk: G labels

AriParkkila deleted the cell-gemalto branch

September 10, 2018 08:26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-version: 5.10.0-rc1