Skip to content

Commit

Permalink
Add protobuf versioning (flipperdevices#954)
Browse files Browse the repository at this point in the history 
* Add protobuf versioning
* Add protobuf version unit test
* Change pb version providing
* Remove redundant 'call'
  • Loading branch information
@albkharisov
albkharisov authored Jan 12, 2022
1 parent 1d31000 commit a0c16e8
Show file tree
Hide file tree
Showing 13 changed files with 164 additions and 34 deletions.
4 changes: 2 additions & 2 deletions applications/rpc/rpc_app.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#include <furi.h>
#include <loader/loader.h>

void rpc_system_app_start_process(const PB_Main* request, void* context) {
static void rpc_system_app_start_process(const PB_Main* request, void* context) {
Rpc* rpc = context;
furi_assert(rpc);
furi_assert(request);
Expand Down Expand Up @@ -36,7 +36,7 @@ void rpc_system_app_start_process(const PB_Main* request, void* context) {
rpc_send_and_release_empty(rpc, request->command_id, result);
}

void rpc_system_app_lock_status_process(const PB_Main* request, void* context) {
static void rpc_system_app_lock_status_process(const PB_Main* request, void* context) {
Rpc* rpc = context;
furi_assert(rpc);
furi_assert(request);
Expand Down
18 changes: 10 additions & 8 deletions applications/rpc/rpc_gui.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ typedef struct {
bool virtual_display_not_empty;
} RpcGuiSystem;

void rpc_system_gui_screen_stream_frame_callback(uint8_t* data, size_t size, void* context) {
static void
rpc_system_gui_screen_stream_frame_callback(uint8_t* data, size_t size, void* context) {
furi_assert(data);
furi_assert(size == 1024);
furi_assert(context);
Expand All @@ -35,7 +36,7 @@ void rpc_system_gui_screen_stream_frame_callback(uint8_t* data, size_t size, voi
free(frame);
}

void rpc_system_gui_start_screen_stream_process(const PB_Main* request, void* context) {
static void rpc_system_gui_start_screen_stream_process(const PB_Main* request, void* context) {
furi_assert(request);
furi_assert(context);
RpcGuiSystem* rpc_gui = context;
Expand All @@ -46,7 +47,7 @@ void rpc_system_gui_start_screen_stream_process(const PB_Main* request, void* co
rpc_gui->gui, rpc_system_gui_screen_stream_frame_callback, context);
}

void rpc_system_gui_stop_screen_stream_process(const PB_Main* request, void* context) {
static void rpc_system_gui_stop_screen_stream_process(const PB_Main* request, void* context) {
furi_assert(request);
furi_assert(context);
RpcGuiSystem* rpc_gui = context;
Expand All @@ -56,7 +57,8 @@ void rpc_system_gui_stop_screen_stream_process(const PB_Main* request, void* con
rpc_send_and_release_empty(rpc_gui->rpc, request->command_id, PB_CommandStatus_OK);
}

void rpc_system_gui_send_input_event_request_process(const PB_Main* request, void* context) {
static void
rpc_system_gui_send_input_event_request_process(const PB_Main* request, void* context) {
furi_assert(request);
furi_assert(request->which_content == PB_Main_gui_send_input_event_request_tag);
furi_assert(context);
Expand Down Expand Up @@ -141,7 +143,7 @@ static void rpc_system_gui_virtual_display_render_callback(Canvas* canvas, void*
canvas_draw_xbm(canvas, 0, 0, canvas->width, canvas->height, rpc_gui->virtual_display_buffer);
}

void rpc_system_gui_start_virtual_display_process(const PB_Main* request, void* context) {
static void rpc_system_gui_start_virtual_display_process(const PB_Main* request, void* context) {
furi_assert(request);
furi_assert(context);
RpcGuiSystem* rpc_gui = context;
Expand Down Expand Up @@ -179,7 +181,7 @@ void rpc_system_gui_start_virtual_display_process(const PB_Main* request, void*
rpc_send_and_release_empty(rpc_gui->rpc, request->command_id, PB_CommandStatus_OK);
}

void rpc_system_gui_stop_virtual_display_process(const PB_Main* request, void* context) {
static void rpc_system_gui_stop_virtual_display_process(const PB_Main* request, void* context) {
furi_assert(request);
furi_assert(context);
RpcGuiSystem* rpc_gui = context;
Expand All @@ -199,7 +201,7 @@ void rpc_system_gui_stop_virtual_display_process(const PB_Main* request, void* c
rpc_send_and_release_empty(rpc_gui->rpc, request->command_id, PB_CommandStatus_OK);
}

void rpc_system_gui_virtual_display_frame_process(const PB_Main* request, void* context) {
static void rpc_system_gui_virtual_display_frame_process(const PB_Main* request, void* context) {
furi_assert(request);
furi_assert(context);
RpcGuiSystem* rpc_gui = context;
Expand Down Expand Up @@ -268,4 +270,4 @@ void rpc_system_gui_free(void* ctx) {
gui_set_framebuffer_callback(rpc_gui->gui, NULL, NULL);
furi_record_close("gui");
free(rpc_gui);
}
}
49 changes: 38 additions & 11 deletions applications/rpc/rpc_system.c
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
#include "flipper.pb.h"
#include "rpc_i.h"

#include <flipper.pb.h>
#include <furi_hal.h>
#include <power/power_service/power.h>
#include <notification/notification_messages.h>
#include <protobuf_version.h>

void rpc_system_system_ping_process(const PB_Main* msg_request, void* context) {
#include "rpc_i.h"

static void rpc_system_system_ping_process(const PB_Main* msg_request, void* context) {
furi_assert(msg_request);
furi_assert(msg_request->which_content == PB_Main_system_ping_request_tag);
furi_assert(context);
Expand Down Expand Up @@ -34,7 +35,7 @@ void rpc_system_system_ping_process(const PB_Main* msg_request, void* context) {
rpc_send_and_release(rpc, &msg_response);
}

void rpc_system_system_reboot_process(const PB_Main* request, void* context) {
static void rpc_system_system_reboot_process(const PB_Main* request, void* context) {
furi_assert(request);
furi_assert(request->which_content == PB_Main_system_reboot_request_tag);
furi_assert(context);
Expand All @@ -57,7 +58,7 @@ typedef struct {
PB_Main* response;
} RpcSystemSystemDeviceInfoContext;

void rpc_system_system_device_info_callback(
static void rpc_system_system_device_info_callback(
const char* key,
const char* value,
bool last,
Expand All @@ -77,7 +78,7 @@ void rpc_system_system_device_info_callback(
rpc_send_and_release(ctx->rpc, ctx->response);
}

void rpc_system_system_device_info_process(const PB_Main* request, void* context) {
static void rpc_system_system_device_info_process(const PB_Main* request, void* context) {
furi_assert(request);
furi_assert(request->which_content == PB_Main_system_device_info_request_tag);
furi_assert(context);
Expand All @@ -98,7 +99,7 @@ void rpc_system_system_device_info_process(const PB_Main* request, void* context
free(response);
}

void rpc_system_system_get_datetime_process(const PB_Main* request, void* context) {
static void rpc_system_system_get_datetime_process(const PB_Main* request, void* context) {
furi_assert(request);
furi_assert(request->which_content == PB_Main_system_get_datetime_request_tag);
furi_assert(context);
Expand All @@ -121,9 +122,10 @@ void rpc_system_system_get_datetime_process(const PB_Main* request, void* contex
response->content.system_get_datetime_response.datetime.weekday = datetime.weekday;

rpc_send_and_release(rpc, response);
free(response);
}

void rpc_system_system_set_datetime_process(const PB_Main* request, void* context) {
static void rpc_system_system_set_datetime_process(const PB_Main* request, void* context) {
furi_assert(request);
furi_assert(request->which_content == PB_Main_system_set_datetime_request_tag);
furi_assert(context);
Expand All @@ -148,7 +150,7 @@ void rpc_system_system_set_datetime_process(const PB_Main* request, void* contex
rpc_send_and_release_empty(rpc, request->command_id, PB_CommandStatus_OK);
}

void rpc_system_system_factory_reset_process(const PB_Main* request, void* context) {
static void rpc_system_system_factory_reset_process(const PB_Main* request, void* context) {
furi_assert(request);
furi_assert(request->which_content == PB_Main_system_factory_reset_request_tag);
furi_assert(context);
Expand All @@ -157,7 +159,8 @@ void rpc_system_system_factory_reset_process(const PB_Main* request, void* conte
power_reboot(PowerBootModeNormal);
}

void rpc_system_system_play_audiovisual_alert_process(const PB_Main* request, void* context) {
static void
rpc_system_system_play_audiovisual_alert_process(const PB_Main* request, void* context) {
furi_assert(request);
furi_assert(request->which_content == PB_Main_system_play_audiovisual_alert_request_tag);
furi_assert(context);
Expand All @@ -170,6 +173,27 @@ void rpc_system_system_play_audiovisual_alert_process(const PB_Main* request, vo
rpc_send_and_release_empty(rpc, request->command_id, PB_CommandStatus_OK);
}

static void rpc_system_system_protobuf_version_process(const PB_Main* request, void* context) {
furi_assert(request);
furi_assert(request->which_content == PB_Main_system_protobuf_version_request_tag);
furi_assert(context);

Rpc* rpc = context;

PB_Main* response = furi_alloc(sizeof(PB_Main));
response->command_id = request->command_id;
response->has_next = false;
response->command_status = PB_CommandStatus_OK;
response->which_content = PB_Main_system_protobuf_version_response_tag;
/* build error here means something wrong with tags in
* local repo https://github.com/flipperdevices/flipperzero-protobuf */
response->content.system_protobuf_version_response.major = PROTOBUF_MAJOR_VERSION;
response->content.system_protobuf_version_response.minor = PROTOBUF_MINOR_VERSION;

rpc_send_and_release(rpc, response);
free(response);
}

void* rpc_system_system_alloc(Rpc* rpc) {
RpcHandler rpc_handler = {
.message_handler = NULL,
Expand Down Expand Up @@ -198,5 +222,8 @@ void* rpc_system_system_alloc(Rpc* rpc) {
rpc_handler.message_handler = rpc_system_system_play_audiovisual_alert_process;
rpc_add_handler(rpc, PB_Main_system_play_audiovisual_alert_request_tag, &rpc_handler);

rpc_handler.message_handler = rpc_system_system_protobuf_version_process;
rpc_add_handler(rpc, PB_Main_system_protobuf_version_request_tag, &rpc_handler);

return NULL;
}
37 changes: 37 additions & 0 deletions applications/tests/rpc/rpc_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <lib/toolbox/md5.h>
#include <cli/cli.h>
#include <loader/loader.h>
#include <protobuf_version.h>

LIST_DEF(MsgList, PB_Main, M_POD_OPLIST)
#define M_OPL_MsgList_t() LIST_OPLIST(MsgList)
Expand Down Expand Up @@ -424,6 +425,15 @@ static void test_rpc_compare_messages(PB_Main* result, PB_Main* expected) {
mu_check(!strcmp(result_md5sum, expected_md5sum));
break;
}
case PB_Main_system_protobuf_version_response_tag: {
uint32_t major_version_expected = expected->content.system_protobuf_version_response.major;
uint32_t minor_version_expected = expected->content.system_protobuf_version_response.minor;
uint32_t major_version_result = result->content.system_protobuf_version_response.major;
uint32_t minor_version_result = result->content.system_protobuf_version_response.minor;
mu_check(major_version_expected == major_version_result);
mu_check(minor_version_expected == minor_version_result);
break;
}
default:
furi_assert(0);
break;
Expand Down Expand Up @@ -1286,6 +1296,32 @@ MU_TEST(test_ping) {
test_rpc_free_msg_list(expected_msg_list);
}

MU_TEST(test_system_protobuf_version) {
MsgList_t expected_msg_list;
MsgList_init(expected_msg_list);

PB_Main request;
request.command_id = ++command_id;
request.command_status = PB_CommandStatus_OK;
request.cb_content.funcs.decode = NULL;
request.has_next = false;
request.which_content = PB_Main_system_protobuf_version_request_tag;

PB_Main* response = MsgList_push_new(expected_msg_list);
response->command_id = command_id;
response->command_status = PB_CommandStatus_OK;
response->cb_content.funcs.encode = NULL;
response->has_next = false;
response->which_content = PB_Main_system_protobuf_version_response_tag;
response->content.system_protobuf_version_response.major = PROTOBUF_MAJOR_VERSION;
response->content.system_protobuf_version_response.minor = PROTOBUF_MINOR_VERSION;

test_rpc_encode_and_feed_one(&request);
test_rpc_decode_and_compare(expected_msg_list);

test_rpc_free_msg_list(expected_msg_list);
}

// TODO: 1) test for rubbish data
// 2) test for unexpected end of packet
// 3) test for one push of several packets
Expand All @@ -1295,6 +1331,7 @@ MU_TEST_SUITE(test_rpc_system) {
MU_SUITE_CONFIGURE(&test_rpc_setup, &test_rpc_teardown);

MU_RUN_TEST(test_ping);
MU_RUN_TEST(test_system_protobuf_version);
}

MU_TEST_SUITE(test_rpc_storage) {
Expand Down
5 changes: 1 addition & 4 deletions assets/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,11 @@ icons: $(ASSETS)
$(PROTOBUF) &: $(PROTOBUF_SOURCES) $(PROTOBUF_COMPILER)
@echo "\tPROTOBUF\t" $(PROTOBUF_FILENAMES)
@$(PROTOBUF_COMPILER) -q -I$(PROTOBUF_SOURCE_DIR) -D$(PROTOBUF_COMPILED_DIR) $(PROTOBUF_SOURCES)
@printf "#pragma once\n#define PROTOBUF_MAJOR_VERSION $(PROTOBUF_MAJOR_VERSION)\n#define PROTOBUF_MINOR_VERSION $(PROTOBUF_MINOR_VERSION)\n" > $(PROTOBUF_COMPILED_DIR)/protobuf_version.h

.PHONY: protobuf
protobuf: $(PROTOBUF)

$(PROTOBUF_FILE_ANIMATIONS): $(PROTOBUF_SOURCES_FILE_ANIMATIONS) $(PROTOBUF_COMPILER)
@echo "\tFILE ANIMATIONS\t" $(PROTOBUF_FILE_ANIMATIONS_FILENAMES)
@$(PROTOBUF_COMPILER) -q -I$(PROTOBUF_FILE_ANIMATIONS_SOURCE_DIR) -D$(PROTOBUF_FILE_ANIMATIONS_COMPILED_DIR) $(PROTOBUF_FILE_ANIMATIONS_SOURCES)

$(DOLPHIN_OUTPUT_DIR): $(DOLPHIN_SOURCE_DIR)
@echo "\tDOLPHIN"
@$(ASSETS_COMPILLER) dolphin "$(DOLPHIN_SOURCE_DIR)" "$(DOLPHIN_OUTPUT_DIR)"
Expand Down
8 changes: 7 additions & 1 deletion assets/assets.mk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@ PROTOBUF_COMPILER := $(PROJECT_ROOT)/lib/nanopb/generator/nanopb_generator.py
PROTOBUF_COMPILED_DIR := $(ASSETS_COMPILED_DIR)
PROTOBUF_SOURCES := $(shell find $(PROTOBUF_SOURCE_DIR) -type f -iname '*.proto')
PROTOBUF_FILENAMES := $(notdir $(addsuffix .pb.c,$(basename $(PROTOBUF_SOURCES))))
PROTOBUF := $(addprefix $(PROTOBUF_COMPILED_DIR)/,$(PROTOBUF_FILENAMES))
PROTOBUF := $(addprefix $(PROTOBUF_COMPILED_DIR)/,$(PROTOBUF_FILENAMES)) $(PROTOBUF_COMPILED_DIR)/protobuf_version.h
PROTOBUF_VERSION := $(shell git -C $(PROTOBUF_SOURCE_DIR) fetch --tags 2>/dev/null && git -C $(PROTOBUF_SOURCE_DIR) describe --tags --abbrev=0 2>/dev/null || echo 'unknown')
PROTOBUF_MAJOR_VERSION := $(word 1,$(subst ., ,$(PROTOBUF_VERSION)))
PROTOBUF_MINOR_VERSION := $(word 2,$(subst ., ,$(PROTOBUF_VERSION)))
$(if $(PROTOBUF_MAJOR_VERSION),,$(error "Protobuf major version is not specified, $$PROTOBUF_VERSION=$(PROTOBUF_VERSION), please perform git fetch in assets/protobuf directory"))
$(if $(PROTOBUF_MINOR_VERSION),,$(error "Protobuf minor version is not specified, $$PROTOBUF_VERSION=$(PROTOBUF_VERSION), please perform git fetch in assets/protobuf directory"))

PROTOBUF_CFLAGS += -DPB_ENABLE_MALLOC

CFLAGS += -I$(ASSETS_COMPILED_DIR) $(PROTOBUF_CFLAGS)
Expand Down
10 changes: 9 additions & 1 deletion assets/compiled/flipper.pb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ typedef struct _PB_Main {
PB_System_GetDateTimeResponse system_get_datetime_response;
PB_System_SetDateTimeRequest system_set_datetime_request;
PB_System_PlayAudiovisualAlertRequest system_play_audiovisual_alert_request;
PB_System_ProtobufVersionRequest system_protobuf_version_request;
PB_System_ProtobufVersionResponse system_protobuf_version_response;
} content;
} PB_Main;

Expand Down Expand Up @@ -157,6 +159,8 @@ extern "C" {
#define PB_Main_system_get_datetime_response_tag 36
#define PB_Main_system_set_datetime_request_tag 37
#define PB_Main_system_play_audiovisual_alert_request_tag 38
#define PB_Main_system_protobuf_version_request_tag 39
#define PB_Main_system_protobuf_version_response_tag 40

/* Struct field encoding specification for nanopb */
#define PB_Empty_FIELDLIST(X, a) \
Expand Down Expand Up @@ -207,7 +211,9 @@ X(a, STATIC, ONEOF, MSG_W_CB, (content,system_factory_reset_request,content
X(a, STATIC, ONEOF, MSG_W_CB, (content,system_get_datetime_request,content.system_get_datetime_request), 35) \
X(a, STATIC, ONEOF, MSG_W_CB, (content,system_get_datetime_response,content.system_get_datetime_response), 36) \
X(a, STATIC, ONEOF, MSG_W_CB, (content,system_set_datetime_request,content.system_set_datetime_request), 37) \
X(a, STATIC, ONEOF, MSG_W_CB, (content,system_play_audiovisual_alert_request,content.system_play_audiovisual_alert_request), 38)
X(a, STATIC, ONEOF, MSG_W_CB, (content,system_play_audiovisual_alert_request,content.system_play_audiovisual_alert_request), 38) \
X(a, STATIC, ONEOF, MSG_W_CB, (content,system_protobuf_version_request,content.system_protobuf_version_request), 39) \
X(a, STATIC, ONEOF, MSG_W_CB, (content,system_protobuf_version_response,content.system_protobuf_version_response), 40)
#define PB_Main_CALLBACK NULL
#define PB_Main_DEFAULT NULL
#define PB_Main_content_empty_MSGTYPE PB_Empty
Expand Down Expand Up @@ -245,6 +251,8 @@ X(a, STATIC, ONEOF, MSG_W_CB, (content,system_play_audiovisual_alert_reques
#define PB_Main_content_system_get_datetime_response_MSGTYPE PB_System_GetDateTimeResponse
#define PB_Main_content_system_set_datetime_request_MSGTYPE PB_System_SetDateTimeRequest
#define PB_Main_content_system_play_audiovisual_alert_request_MSGTYPE PB_System_PlayAudiovisualAlertRequest
#define PB_Main_content_system_protobuf_version_request_MSGTYPE PB_System_ProtobufVersionRequest
#define PB_Main_content_system_protobuf_version_response_MSGTYPE PB_System_ProtobufVersionResponse

extern const pb_msgdesc_t PB_Empty_msg;
extern const pb_msgdesc_t PB_StopSession_msg;
Expand Down
3 changes: 3 additions & 0 deletions assets/compiled/protobuf_version.h
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#pragma once
#define PROTOBUF_MAJOR_VERSION 0
#define PROTOBUF_MINOR_VERSION 1
6 changes: 6 additions & 0 deletions assets/compiled/system.pb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,11 @@ PB_BIND(PB_System_DateTime, PB_System_DateTime, AUTO)
PB_BIND(PB_System_PlayAudiovisualAlertRequest, PB_System_PlayAudiovisualAlertRequest, AUTO)


PB_BIND(PB_System_ProtobufVersionRequest, PB_System_ProtobufVersionRequest, AUTO)


PB_BIND(PB_System_ProtobufVersionResponse, PB_System_ProtobufVersionResponse, AUTO)




Loading

0 comments on commit a0c16e8

Please sign in to comment.