Skip to content

Conversation

jeromecoutant
Copy link
Collaborator

Summary of changes

It could be useful to get in logs modem module version.

@LMESTM
@AriParkkila

Impact of changes

No impact with default compilation

Example with STMOD_CELLULAR when mbed-trace is enabled:

[INFO][CELL]: Modem manufacturer: Quectel
[INFO][CELL]: Modem model: BG96
[INFO][CELL]: Modem revision: BG96MAR02A07M1G

Migration actions required

Documentation


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers


@ciarmcom ciarmcom requested review from a team January 30, 2020 16:00
@ciarmcom
Copy link
Member

@jeromecoutant, thank you for your changes.
@ARMmbed/mbed-os-wan @ARMmbed/mbed-os-maintainers please review.

}

#if MBED_CONF_MBED_TRACE_ENABLE
CellularInformation *info = _cellularDevice.open_information();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be done only when moving to next state (STATE_SIM_PIN)? We might be restarting/powering or failing to communicate with the modem.

Choose a reason for hiding this comment

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

Yes, this should probably be just before if (device_ready()) at line 364.


#if MBED_CONF_MBED_TRACE_ENABLE
CellularInformation *info = _cellularDevice.open_information();
char buf[80]; // this should actually be 2048, but then you should use heap memory or likely run out of the stack

Choose a reason for hiding this comment

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

This could be something like:

char *buf = new (std::nothrow) char[2048]; // size from 3GPP TS 27.007
if (buf) {
  if (info->get_manufacturer..
  ...
  delete[] buf;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Copy link

@AriParkkila AriParkkila left a comment

Choose a reason for hiding this comment

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

@jeromecoutant Can you change this to use heap memory with new/delete?

APN and registration prints seems to duplicate what you see with the current traces and AT-debug enabled so they are probably not needed here?

@0xc0170 0xc0170 added the release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0 label Feb 7, 2020
@mergify mergify bot added needs: CI and removed needs: work labels Feb 7, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Feb 7, 2020

CI started

@mbed-ci
Copy link

mbed-ci commented Feb 7, 2020

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-IAR
  • jenkins-ci/mbed-os-ci_build-GCC_ARM
  • jenkins-ci/mbed-os-ci_build-ARM

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 10, 2020

Internal CI error, restarted

@mbed-ci
Copy link

mbed-ci commented Feb 10, 2020

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

@mergify mergify bot added needs: work and removed needs: CI labels Feb 10, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Feb 13, 2020

Test restarted

@0xc0170 0xc0170 merged commit d7f3341 into ARMmbed:master Feb 13, 2020
@mergify mergify bot removed the ready for merge label Feb 13, 2020
@jeromecoutant jeromecoutant deleted the PR_MODEMVERSION branch February 14, 2020 16:25
@adbridge
Copy link
Contributor

@AnttiKauppila @AriParkkila Doesn't this require updates to the documentation to detail the changes to displayed information ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants