nanohub HAL: load nanoapp without nanohub reboot
send ENABLE_APP instead of REBOOT; as a side effect
we should now return failure in case app_init() method failed
fix couple logging issues and incorrect response handlings.
Test: execute the sequence below
Note: this requires that Nanohub OS image is built with the following
https://googleplex-android-review.git.corp.google.com/#/c/1503793/
and uploaded to context hub MCU.
As of this moment, this change is source-merged, but binary OS image is
not in sync with it.
With updated OS, execute:
adb shell 'echo 1 > /sys/class/nanohub/nanohub/erase_shared'
adb shell nanoapp_cmd download
adb shell 'echo 1 > /sys/class/nanohub/nanohub/reset'
gts-tradefed run everything --skip-system-status-check
com.android.compatibility.common.tradefed.targetprep.NetworkConnectivityChecker
--module GtsLocationTestCases --test
com.google.android.location.gts.ContextHubHelloWorldNanoAppTest#runTest
--primary-abi-only
gts-tradefed run everything --skip-system-status-check
com.android.compatibility.common.tradefed.targetprep.NetworkConnectivityChecker
--module GtsLocationTestCases --test
com.google.android.location.gts.ContextHubTrivialNanoAppsTest
--primary-abi-only
Bug: 31038843
Bug: 31360590
Change-Id: If0dfe75f7cfa8ddeeabfc6ae124aa4cd155a30a8
Signed-off-by: Alexey Polyudov <apolyudov@google.com>
diff --git a/contexthubhal/system_comms.cpp b/contexthubhal/system_comms.cpp
index 840607a..c4e9b62 100644
--- a/contexthubhal/system_comms.cpp
+++ b/contexthubhal/system_comms.cpp
@@ -281,16 +281,23 @@
return setupMgmt(appMsg, NANOHUB_EXT_APPS_OFF);
case CONTEXT_HUB_UNLOAD_APP:
return setupMgmt(appMsg, NANOHUB_EXT_APP_DELETE);
- case CONTEXT_HUB_LOAD_OS:
case CONTEXT_HUB_LOAD_APP:
+ {
mData.clear();
mData = std::vector<uint8_t>(msgData, msgData + mLen);
+ const load_app_request_t *appReq = static_cast<const load_app_request_t*>(appMsg->message);
+ if (appReq == nullptr || mLen <= sizeof(*appReq)) {
+ ALOGE("%s: Invalid app header: too short\n", __func__);
+ return -EINVAL;
+ }
+ mAppName = appReq->app_binary.app_id;
setState(TRANSFER);
buf.writeU8(NANOHUB_START_UPLOAD);
- buf.writeU8(mCmd == CONTEXT_HUB_LOAD_OS ? 1 : 0);
+ buf.writeU8(0);
buf.writeU32(mLen);
return sendToSystem(buf.getData(), buf.getPos());
+ }
case CONTEXT_HUB_OS_REBOOT:
setState(REBOOT);
@@ -330,8 +337,11 @@
case FINISH:
ret = handleFinish(rsp);
break;
- case RELOAD:
- ret = handleReload(rsp);
+ case RUN:
+ ret = handleRun(rsp);
+ break;
+ case RUN_FAILED:
+ ret = handleRunFailed(rsp);
break;
case REBOOT:
ret = handleReboot(rsp);
@@ -386,9 +396,9 @@
if (success) {
char data[MAX_RX_PACKET];
MessageBuf buf(data, sizeof(data));
- // until app header is passed, we don't know who to start, so we reboot
- buf.writeU8(NANOHUB_REBOOT);
- setState(RELOAD);
+ buf.writeU8(NANOHUB_EXT_APPS_ON);
+ writeAppName(buf, mAppName);
+ setState(RUN);
ret = sendToSystem(buf.getData(), buf.getPos());
} else {
int32_t result = NANOHUB_APP_NOT_LOADED;
@@ -400,15 +410,48 @@
return ret;
}
-/* reboot notification, when triggered as part of App reload sequence */
-int SystemComm::AppMgmtSession::handleReload(NanohubRsp &rsp)
+int SystemComm::AppMgmtSession::handleRun(NanohubRsp &rsp)
{
- int32_t result = NANOHUB_APP_LOADED;
+ if (rsp.cmd != NANOHUB_EXT_APPS_ON)
+ return 1;
- ALOGI("Nanohub reboot status [NEW APP START]: %08" PRIX32, rsp.status);
+ MgmtStatus sts = { .value = (uint32_t)rsp.status };
+
+ // op counter returns number of nanoapps that were started as result of the command
+ // for successful start command it must be > 0
+ int32_t result = sts.value > 0 && sts.op > 0 && sts.op <= 0x7F ? 0 : -1;
+
+ ALOGI("Nanohub NEW APP START: %08" PRIX32 "\n", rsp.status);
+ if (result != 0) {
+ // if nanoapp failed to start we have to unload it
+ char data[MAX_RX_PACKET];
+ MessageBuf buf(data, sizeof(data));
+ buf.writeU8(NANOHUB_EXT_APP_DELETE);
+ writeAppName(buf, mAppName);
+ if (sendToSystem(buf.getData(), buf.getPos()) == 0) {
+ setState(RUN_FAILED);
+ return 0;
+ }
+ ALOGE("%s: failed to send DELETE for failed app\n", __func__);
+ }
+
+ // it is either success, and we report it, or
+ // it is a failure to load, and also failure to send erase command
+ sendToApp(mCmd, &result, sizeof(result));
+ complete();
+ return 0;
+}
+
+int SystemComm::AppMgmtSession::handleRunFailed(NanohubRsp &rsp)
+{
+ if (rsp.cmd != NANOHUB_EXT_APP_DELETE)
+ return 1;
+
+ int32_t result = -1;
+
+ ALOGI("%s: APP DELETE [because it failed]: %08" PRIX32 "\n", __func__, rsp.status);
sendToApp(mCmd, &result, sizeof(result));
-
complete();
return 0;
@@ -417,7 +460,9 @@
/* reboot notification, when triggered by App request */
int SystemComm::AppMgmtSession::handleReboot(NanohubRsp &rsp)
{
- ALOGI("Nanohub reboot status [USER REQ]: %08" PRIX32, rsp.status);
+ if (rsp.cmd != NANOHUB_REBOOT)
+ return 1;
+ ALOGI("Nanohub reboot status [USER REQ]: %08" PRIX32 "\n", rsp.status);
// reboot notification is sent by SessionManager
complete();
@@ -429,7 +474,6 @@
{
bool valid = false;
- ALOGI("Nanohub MGMT response: CMD=%02X; STATUS=%08" PRIX32, rsp.cmd, rsp.status);
int32_t result = rsp.status;
// TODO: remove this when context hub service can handle non-zero success status
@@ -455,6 +499,7 @@
return 1;
}
+ ALOGI("Nanohub MGMT response: CMD=%02X; STATUS=%08" PRIX32, rsp.cmd, rsp.status);
if (!valid) {
ALOGE("Invalid response for this state: APP CMD=%02X", mCmd);
return -EINVAL;
diff --git a/contexthubhal/system_comms.h b/contexthubhal/system_comms.h
index 256c8d4..e1b08e3 100644
--- a/contexthubhal/system_comms.h
+++ b/contexthubhal/system_comms.h
@@ -46,10 +46,6 @@
#define NANOHUB_FINISH_UPLOAD 8 // () -> (char success)
#define NANOHUB_REBOOT 9 // () -> (char success)
-// Custom defined private messages
-#define CONTEXT_HUB_LOAD_OS (CONTEXT_HUB_TYPE_PRIVATE_MSG_BASE + 1)
-
-
#define NANOHUB_APP_NOT_LOADED (-1)
#define NANOHUB_APP_LOADED (0)
@@ -68,6 +64,18 @@
uint32_t version, flashUse, ramUse;
} __attribute__((packed));
+struct MgmtStatus {
+ union {
+ uint32_t value;
+ struct {
+ uint8_t app;
+ uint8_t task;
+ uint8_t op;
+ uint8_t erase;
+ } __attribute__((packed));
+ };
+} __attribute__((packed));
+
struct NanohubMemInfo {
//sizes
uint32_t flashSz, blSz, osSz, sharedSz, eeSz;
@@ -186,20 +194,23 @@
enum {
TRANSFER = SESSION_USER,
FINISH,
- RELOAD,
+ RUN,
+ RUN_FAILED,
REBOOT,
MGMT,
};
- uint32_t mCmd; // UPLOAD_APP | UPPLOAD_OS
+ uint32_t mCmd; // LOAD_APP, UNLOAD_APP, ENABLE_APP, DISABLE_APP
uint32_t mResult;
std::vector<uint8_t> mData;
uint32_t mLen;
uint32_t mPos;
+ hub_app_name_t mAppName;
int setupMgmt(const hub_message_t *appMsg, uint32_t cmd);
int handleTransfer(NanohubRsp &rsp);
int handleFinish(NanohubRsp &rsp);
- int handleReload(NanohubRsp &rsp);
+ int handleRun(NanohubRsp &rsp);
+ int handleRunFailed(NanohubRsp &rsp);
int handleReboot(NanohubRsp &rsp);
int handleMgmt(NanohubRsp &rsp);
public:
@@ -208,6 +219,7 @@
mResult = 0;
mPos = 0;
mLen = 0;
+ memset(&mAppName, 0, sizeof(mAppName));
}
virtual int handleRx(MessageBuf &buf) override;
virtual int setup(const hub_message_t *app_msg) override;
