Skip to content

Commit

Permalink
Finish flexible length config packet
Browse files Browse the repository at this point in the history
  • Loading branch information
@Apehaenger
Apehaenger committed Oct 10, 2024
1 parent f3936e6 commit d90b6ca
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 85 deletions.
25 changes: 14 additions & 11 deletions Firmware/LowLevel/src/datatypes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,31 +142,34 @@ struct ll_ui_event {

typedef char iso639_1[2]; // Two char ISO 639-1 language code

enum class HallMode : unsigned int {
enum class HallMode : uint8_t {
off = 0,
emergency, // lift & tilt
stop,
pause
};

// FIXME: Decide later which is more comfortable activeLow = 0 | 1
enum class HallLevel : unsigned int {
// FIXME: Decide later which is more comfortable, activeLow = 0 | 1
enum class HallLevel : uint8_t {
activeLow = 0, // If Hall-Sensor (or button) is active/triggered we've this level on our GPIO
activeHigh
};

#pragma pack(push, 1)
struct HallConfig {
HallMode mode : 4 {0};
HallLevel level : 1 {0};
};
HallMode mode : 4;
HallLevel level : 1;
} __attribute__((packed));
#pragma pack(pop)

#define MAX_HALL_INPUTS 14 // How much Hall-inputs we support. 4 * OM + 6 * Stock-CoverUI + 4 spare
#define MAX_HALL_INPUTS 10 // How much Hall-inputs we support. 4 * OM + 6 * Stock-CoverUI + 0 spare (because not yet required to make it fixed)

// LL/HL config packet, bi-directional, flexible-length, with defaults for YF-C500
// LL/HL config packet, encapsulated in wire_msg for transfer, bi-directional, flexible-length, with defaults for YF-C500.
#pragma pack(push, 1)
struct ll_high_level_config {
uint8_t type;
uint16_t crc;
// ATTENTION: This is a flexible length struct. It is allowed to grow independently to HL without loosing compatibility,
// but never change or restructure already published member, except you really know their consequences.
uint8_t comms_version = 1; // Transitional comms_version, can safely renamed as spare for other use in, let's say 2025 ;-)
uint8_t config_bitmask = 0; // See LL_HIGH_LEVEL_CONFIG_BIT_*
int8_t volume = 80; // Volume (0-100%) feedback (if directly changed via CoverUI)
iso639_1 language = {'e', 'n'}; // ISO 639-1 (2-char) language code (en, de, ...)
Expand All @@ -182,7 +185,7 @@ struct ll_high_level_config {
{HallMode::stop, HallLevel::activeLow}, // [3] OM Hall-4 input
// [4] Stock-CoverUI-1 ... [9] Stock-CoverUI-6 defaults to off
};
// INFO: Before adding a new member: Do we need to add some hall_config spares?
// INFO: Before adding a new member here: Decide if and how much hall_configs spares do we like to have
} __attribute__((packed));
#pragma pack(pop)

Expand Down
107 changes: 33 additions & 74 deletions Firmware/LowLevel/src/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,9 @@ uint16_t ui_version = 0; // Last received UI firmware version
uint8_t ui_topic_bitmask = Topic_set_leds; // UI subscription, default to Set_LEDs
uint16_t ui_interval = 1000; // UI send msg (LED/State) interval (ms)

struct ll_high_level_config llhl_config; // LL/HL configuration (get initialized with YF-C500 defaults)
struct ll_high_level_config llhl_config; // LL/HL configuration (is initialized with YF-C500 defaults)
nv_config::Config *nv_cfg; // Non-volatile configuration

// Some vars related to PACKET_ID_LL_HIGH_LEVEL_CONFIG_*
uint8_t comms_version = 0; // comms packet version (>0 if implemented)
uint8_t config_bitmask = 0; // See LL_HIGH_LEVEL_CONFIG_BIT_*

void sendMessage(void *message, size_t size);
void sendUIMessage(void *message, size_t size);
void onPacketReceived(const uint8_t *buffer, size_t size);
Expand Down Expand Up @@ -552,12 +548,14 @@ void onUIPacketReceived(const uint8_t *buffer, size_t size) {
}

void sendConfigMessage(uint8_t pkt_type) {
struct ll_high_level_config ll_config;
ll_config.type = pkt_type;
ll_config.config_bitmask = config_bitmask;
ll_config.volume = 80; // FIXME: Adapt once nv_config or improve-sound got merged
strncpy(ll_config.language, "en", 2); // FIXME: Adapt once nv_config or improve-sound got merged
sendMessage(&ll_config, sizeof(struct ll_high_level_config));
size_t msg_size = sizeof(struct ll_high_level_config) + 3; // + 1 type + 2 crc
uint8_t *msg = (uint8_t *)malloc(msg_size);
if (msg == NULL)
return; // malloc() failed
*msg = pkt_type;
memcpy(msg + 1, &llhl_config, sizeof(struct ll_high_level_config)); // Copy our live config into the message, behind type
sendMessage(msg, msg_size);
free(msg);
}

void onPacketReceived(const uint8_t *buffer, size_t size) {
Expand All @@ -566,37 +564,11 @@ void onPacketReceived(const uint8_t *buffer, size_t size) {
return;

// check the CRC
uint16_t crc;

// @ClemensElflein: Here is why I decided against having CRC at first packet member:
// We need to select between the two CRC positions. But if CRC would be on first place, type would go somewhere behind it.
// But then type for this packet would be on a position where the other packets place their data(or CRC). This would
// quickly result in wrong packet-type identifications.
// Could also be solved, but would complicate code and double CRC calculations for the packets which got identified wrong.
// The alternative I do see could be a cascaded CRC calculation:
// Nearly 99.99% of all packets have their packet at the end, so we always could try that first, but when it fails,
// we could fall back and try with CRC on first position to see if it's this one-time packet.
// But I would prefer this more strict and defined way.

// We have two CRC position variants
if (buffer[0] == PACKET_ID_LL_HIGH_LEVEL_CONFIG_REQ || buffer[0] == PACKET_ID_LL_HIGH_LEVEL_CONFIG_RSP) {
// Flexible length packet, where the CRC follows the type and type is NOT part of the CRC.

// Comment: This is just as safe as including the type into the CRC because: If type would be accidentally wrong,
// then it also wouldn't validate in the "else" CRC calculation where type is included
crc = CRC16.ccitt(buffer + 3, size - 3);

if (buffer[1] != ((crc >> 8) & 0xFF) ||
buffer[2] != (crc & 0xFF))
return;
} else {
// Normal, fixed length packet, where the CRC is the last member
crc = CRC16.ccitt(buffer, size - 2);
uint16_t crc = CRC16.ccitt(buffer, size - 2);

if (buffer[size - 1] != ((crc >> 8) & 0xFF) ||
buffer[size - 2] != (crc & 0xFF))
return;
}
if (buffer[size - 1] != ((crc >> 8) & 0xFF) ||
buffer[size - 2] != (crc & 0xFF))
return;

if (buffer[0] == PACKET_ID_LL_HEARTBEAT && size == sizeof(struct ll_heartbeat)) {

Expand All @@ -620,40 +592,27 @@ void onPacketReceived(const uint8_t *buffer, size_t size) {
} else if (buffer[0] == PACKET_ID_LL_HIGH_LEVEL_STATE && size == sizeof(struct ll_high_level_state)) {
// copy the state
last_high_level_state = *((struct ll_high_level_state *) buffer);
} else if ((buffer[0] == PACKET_ID_LL_HIGH_LEVEL_CONFIG_REQ || buffer[0] == PACKET_ID_LL_HIGH_LEVEL_CONFIG_RSP) &&
(size == sizeof(struct ll_high_level_config) || size == sizeof(struct ll_high_level_config) + sizeof(struct ll_high_level_config_ext_v2))) {
// Read and handle received config
struct ll_high_level_config *pkt = (struct ll_high_level_config *)buffer;
// Lower comms_version is leading
pkt->comms_version <= LL_HIGH_LEVEL_CONFIG_MAX_COMMS_VERSION ? comms_version = pkt->comms_version : comms_version = LL_HIGH_LEVEL_CONFIG_MAX_COMMS_VERSION;
config_bitmask = pkt->config_bitmask; // Take over as sent. HL is leading (for now)
} else if (buffer[0] == PACKET_ID_LL_HIGH_LEVEL_CONFIG_REQ || buffer[0] == PACKET_ID_LL_HIGH_LEVEL_CONFIG_RSP) {
// This is a flexible length package where the size may vary when ll_high_level_config struct got enhanced only on one side.
// If payload size is larger than our struct size, ensure that we only copy those we know of = our struct size.
// If payload size is smaller than our struct size, copy only the payload we got, but ensure that the unsent member(s) have reasonable defaults.
size_t payload_size = min(sizeof(ll_high_level_config), size - 3); // -1 type -2 crc

// PR-80: Assign volume & language if not already stored in flash-config
// Use a temporary config for easier sanity adaption and copy our live config, which has at least reasonable defaults.
// The live config copy ensures that we've reasonable values for the case that HL config struct is older than ours.
ll_high_level_config tmp_config = llhl_config;

// Handle possible ll_high_level_config_ext_v2 extension
if (comms_version >= 2) {
// Read and handle extension v2
struct ll_high_level_config_ext_v2 *pkt = (struct ll_high_level_config_ext_v2 *)buffer + sizeof(struct ll_high_level_config);

// Fix exceed of absolute max. values
pkt->v_charge_max = min(pkt->v_charge_max, V_CHARGE_ABS_MAX);
pkt->i_charge_max = min(pkt->i_charge_max, I_CHARGE_ABS_MAX);

// Copy extension packet to config_v2
config_v2 = *((struct ll_high_level_config_ext_v2 *)buffer + sizeof(struct ll_high_level_config));

// TODO: Parse hall config fields and build a compact iterable vector/array/... of active hall inputs or do it directly in updateEmergency()

// TODO: Decide which fields should get stored in nv_config (flash) so that it's directly avail after power-up (without the need of ros_running)
// My current thoughts are:
// v_charge_max for those who need a much lower or higher charging voltage, so that battery doesn't get empty or overload if ROS is offline for a longer period (or silently crashed)
// i_charge is similar. If one has a higher current DCDC and dock his empty mower, it should charge, even if ROS is down
// v_battery_max for sure (overcharge protection)
//
// but NOT:
// halls_config/inverted? Think we should always start with default OM halls config but if comms_version >= 2, wait till v2 packet extension got received.
// tilt/lift periods
}
// Copy payload to temporary config (behind type)
memcpy(&tmp_config, buffer + 1, payload_size);

// Sanity
tmp_config.v_charge_max = min(tmp_config.v_charge_max, V_CHARGE_ABS_MAX); // Fix exceed of hardware limits
tmp_config.i_charge_max = min(tmp_config.i_charge_max, I_CHARGE_ABS_MAX); // Fix exceed of hardware limits

// Make config live
llhl_config = tmp_config;

// PR-80: Assign volume & language if not already stored in flash-config

if (buffer[0] == PACKET_ID_LL_HIGH_LEVEL_CONFIG_REQ)
sendConfigMessage(PACKET_ID_LL_HIGH_LEVEL_CONFIG_RSP);
Expand All @@ -662,7 +621,7 @@ void onPacketReceived(const uint8_t *buffer, size_t size) {

// returns true, if it's a good idea to charge the battery (current, voltages, ...)
bool checkShouldCharge() {
return status_message.v_charge < config_v2.v_charge_max && status_message.charging_current < config_v2.i_charge_max && status_message.v_battery < config_v2.v_battery_max;
return status_message.v_charge < llhl_config.v_charge_max && status_message.charging_current < llhl_config.i_charge_max && status_message.v_battery < llhl_config.v_battery_max;
}

void updateChargingEnabled() {
Expand Down

0 comments on commit d90b6ca

Please sign in to comment.