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.

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

Working AT driver for MC7455 and EM7590, not tested with anything else #139

Closed
wants to merge 3 commits into from
Closed

Working AT driver for MC7455 and EM7590, not tested with anything else #139

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
259d28b
Working AT driver for MC7455 and EM7590, not tested with anything else
urielka Aug 2, 2024
1f64a78
Revert long to int in apdu_interface_logic_channel_open
urielka Aug 2, 2024
5fb9ebd
Update driver/apdu/at.c
urielka Aug 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions driver/apdu/at.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@

#define AT_BUFFER_SIZE 20480
static FILE *fuart;
static int logic_channel = 0;

static long logic_channel = 0;
urielka marked this conversation as resolved.
Show resolved Hide resolved
static char *buffer;

static int at_expect(char **response, const char *expected)
Expand Down Expand Up @@ -74,13 +75,15 @@ static int apdu_interface_connect(struct euicc_ctx *ctx)
fprintf(stderr, "Device missing AT+CCHC support\n");
return -1;
}
/* AT+CGLA=? might not be available but AT+CGLA= is
Sierra wireless MC7455 supports both but EM7590 only the latter
fprintf(fuart, "AT+CGLA=?\r\n");
if (at_expect(NULL, NULL))
{
fprintf(stderr, "Device missing AT+CGLA support\n");
return -1;
}

*/
Comment on lines +78 to +86
Copy link
Contributor

Choose a reason for hiding this comment

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

If you remove the check on AT+CGLA=? because it doesn't work, you should add a check on AT+CGLA= or do some error handling when using AT+CGLA=.

Copy link
Author

Choose a reason for hiding this comment

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

I don't think it is that important to be honest, it will just fail after the first command, checking for AT+CGLA= requires to actually do AT+CCHO and then AT+CGLA so what are we gaining here.

Maybe change it to a warning? and again the issue is the lack of AT+CGLA= and not AT+CGLA=? where in AT commands those are separated commands so just like in the case of sierra wireless it might not be implemented

return 0;
}

Expand All @@ -106,7 +109,7 @@ static int apdu_interface_transmit(struct euicc_ctx *ctx, uint8_t **rx, uint32_t
return -1;
}

fprintf(fuart, "AT+CGLA=%d,%u,\"", logic_channel, tx_len * 2);
fprintf(fuart, "AT+CGLA=%lx,%u,\"", logic_channel, tx_len * 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fprintf(fuart, "AT+CGLA=%lx,%u,\"", logic_channel, tx_len * 2);
fprintf(fuart, "AT+CGLA=%" PRIu ",%u,\"", logic_channel, tx_len * 2);

If you choose to use fixed-width integer type.

for (uint32_t i = 0; i < tx_len; i++)
{
fprintf(fuart, "%02X", (uint8_t)(tx[i] & 0xFF));
Expand Down Expand Up @@ -168,11 +171,13 @@ static int apdu_interface_logic_channel_open(struct euicc_ctx *ctx, const uint8_
return logic_channel;
}

/* this code is broken, you can't assume there are 4 open channels
for (int i = 1; i <= 4; i++)
{
fprintf(fuart, "AT+CCHC=%d\r\n", i);
at_expect(NULL, NULL);
}
}*/

fprintf(fuart, "AT+CCHO=\"");
for (int i = 0; i < aid_len; i++)
{
Expand All @@ -187,9 +192,13 @@ static int apdu_interface_logic_channel_open(struct euicc_ctx *ctx, const uint8_
{
return -1;
}
logic_channel = atoi(response);

return logic_channel;
logic_channel = strtol(response, NULL, 16);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
logic_channel = strtol(response, NULL, 16);
logic_channel = strtoull(response, NULL, 16);

Same as above.


// always return 0 as the actual logic channel to eUICC is only available to the modem
// CLA should always be 0x80 see:
// https://github.com/estkme-group/lpac/issues/138
return 0;
}

static void apdu_interface_logic_channel_close(struct euicc_ctx *ctx, uint8_t channel)
Expand All @@ -198,7 +207,7 @@ static void apdu_interface_logic_channel_close(struct euicc_ctx *ctx, uint8_t ch
{
return;
}
fprintf(fuart, "AT+CCHC=%d\r\n", logic_channel);
fprintf(fuart, "AT+CCHC=%lx\r\n", logic_channel);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fprintf(fuart, "AT+CCHC=%lx\r\n", logic_channel);
fprintf(fuart, "AT+CCHC=%" PRIu "\r\n", logic_channel);

Same as above.

at_expect(NULL, NULL);
}

Expand Down