Rewrite all units and other logic to better use return values and added TRY() helper

This commit is contained in:
2018-01-04 11:50:05 +01:00
parent 465242a0f1
commit ab2dfe3a76
22 changed files with 369 additions and 362 deletions
+10 -19
View File
@@ -42,7 +42,7 @@ void rsc_init_registry(void)
* @param rsc - resource to claim
* @return true on successful claim
*/
bool rsc_claim(Unit *unit, Resource rsc)
error_t rsc_claim(Unit *unit, Resource rsc)
{
assert_param(rsc_initialized);
assert_param(rsc > R_NONE && rsc < R_RESOURCE_COUNT);
@@ -53,12 +53,11 @@ bool rsc_claim(Unit *unit, Resource rsc)
dbg("ERROR!! Unit %s failed to claim resource %s, already held by %s!",
unit->name, rsc_names[rsc], resources[rsc].owner->name);
unit->status = E_RESOURCE_NOT_AVAILABLE;
return false;
return E_RESOURCE_NOT_AVAILABLE;
}
resources[rsc].owner = unit;
return true;
return E_SUCCESS;
}
/**
@@ -69,7 +68,7 @@ bool rsc_claim(Unit *unit, Resource rsc)
* @param rsc1 - last resource to claim
* @return true on complete claim, false if any failed (none are claimed in that case)
*/
bool rsc_claim_range(Unit *unit, Resource rsc0, Resource rsc1)
error_t rsc_claim_range(Unit *unit, Resource rsc0, Resource rsc1)
{
assert_param(rsc_initialized);
assert_param(rsc0 > R_NONE && rsc0 < R_RESOURCE_COUNT);
@@ -77,33 +76,25 @@ bool rsc_claim_range(Unit *unit, Resource rsc0, Resource rsc1)
assert_param(unit != NULL);
for (int i = rsc0; i <= rsc1; i++) {
if (!rsc_claim(unit, (Resource) i)) return false;
TRY(rsc_claim(unit, (Resource) i));
}
return true;
return E_SUCCESS;
}
bool rsc_claim_gpios(Unit *unit, char port_name, uint16_t pins)
error_t rsc_claim_gpios(Unit *unit, char port_name, uint16_t pins)
{
bool suc = true;
for (int i = 0; i < 16; i++) {
if (pins & (1 << i)) {
Resource rsc = pin2resource(port_name, (uint8_t) i, &suc);
if (!suc) {
unit->status = E_BAD_CONFIG;
// rsc_teardown(unit);
return false;
}
if (!suc) return E_BAD_CONFIG;
suc = rsc_claim(unit, rsc);
if (!suc) {
// rsc_teardown(unit);
return false;
}
TRY(rsc_claim(unit, rsc));
}
}
return true;
return E_SUCCESS;
}
/**
+7 -8
View File
@@ -81,13 +81,8 @@ enum hw_resource {
void rsc_init_registry(void);
bool rsc_claim(Unit *unit, Resource rsc);
bool rsc_claim_range(Unit *unit, Resource rsc0, Resource rsc1);
void rsc_teardown(Unit *unit);
void rsc_free(Unit *unit, Resource rsc);
void rsc_free_range(Unit *unit, Resource rsc0, Resource rsc1);
error_t rsc_claim(Unit *unit, Resource rsc);
error_t rsc_claim_range(Unit *unit, Resource rsc0, Resource rsc1);
/**
* Claim GPIOs by bitmask and port name, atomically.
* Tear down the unit on failure.
@@ -97,6 +92,10 @@ void rsc_free_range(Unit *unit, Resource rsc0, Resource rsc1);
* @param pins - pins, bitmask
* @return success
*/
bool rsc_claim_gpios(Unit *unit, char port_name, uint16_t pins);
error_t rsc_claim_gpios(Unit *unit, char port_name, uint16_t pins);
void rsc_teardown(Unit *unit);
void rsc_free(Unit *unit, Resource rsc);
void rsc_free_range(Unit *unit, Resource rsc0, Resource rsc1);
#endif //GEX_RESOURCES_H
+3 -2
View File
@@ -308,8 +308,9 @@ void settings_load_ini_key(const char *restrict section, const char *restrict ke
namebuf[csptr - nameptr - 1] = 0;
uint8_t cs = (uint8_t) avr_atoi(csptr + 1);
bool res = ureg_load_unit_ini_key(namebuf, key, value, cs);
if (!res) dbg("!! error loading %s@%d.%s = %s", namebuf, (int)cs, key, value);
error_t rv = ureg_load_unit_ini_key(namebuf, key, value, cs);
if (rv != E_SUCCESS)
dbg("!! error loading %s@%d.%s = %s - error %s", namebuf, (int)cs, key, value, error_get_message(rv));
} else {
dbg("! Bad config key: [%s] %s = %s", section, key, value);
}
+4 -5
View File
@@ -48,7 +48,7 @@ struct unit_driver {
/**
* Pre-init: allocate data object, init defaults
*/
bool (*preInit)(Unit *unit);
error_t (*preInit)(Unit *unit);
/**
* Load settings from binary storage, parse and store them in the data object.
@@ -73,11 +73,10 @@ struct unit_driver {
* @param key - key from the INI file
* @param value - value from the ini file; strings have already removed quotes and replaced escape sequences with ASCII as needed
*/
bool (*cfgLoadIni)(Unit *unit, const char *key, const char *value);
error_t (*cfgLoadIni)(Unit *unit, const char *key, const char *value);
/**
* Export settings to a INI file.
* Capacity will likely be 512 bytes, do not waste space!
*
* @param buffer - destination buffer
* @param capacity - buffer size
@@ -88,7 +87,7 @@ struct unit_driver {
/**
* Finalize the init sequence, validate settings, enable peripherals and prepare for operation
*/
bool (*init)(Unit *unit);
error_t (*init)(Unit *unit);
/**
* De-initialize the unit: de-init peripheral, free resources, free data object...
@@ -99,7 +98,7 @@ struct unit_driver {
/**
* Handle an incoming request. Return true if command was OK.
*/
bool (*handleRequest)(Unit *unit, TF_ID frame_id, uint8_t command, PayloadParser *pp);
error_t (*handleRequest)(Unit *unit, TF_ID frame_id, uint8_t command, PayloadParser *pp);
};
/**
+29 -29
View File
@@ -100,6 +100,7 @@ static void add_unit_to_list(UlistEntry *le)
Unit *ureg_instantiate(const char *driver_name)
{
bool suc = true;
error_t rv;
// Find type in the repository
UregEntry *re = ureg_head;
@@ -113,19 +114,19 @@ Unit *ureg_instantiate(const char *driver_name)
Unit *pUnit = &le->unit;
pUnit->driver = re->driver;
pUnit->status = E_LOADING;
pUnit->status = E_LOADING; // indeterminate default state
pUnit->data = NULL;
pUnit->callsign = 0;
suc = pUnit->driver->preInit(pUnit);
if (!suc) {
rv = pUnit->driver->preInit(pUnit);
if (rv != E_SUCCESS) {
// tear down what we already allocated and abort
// If it failed this early, the only plausible explanation is failed malloc,
// in which case the data structure is not populated and keeping the
// broken unit doesn't serve any purpose. Just ditch it...
dbg("!! Unit type %s failed to pre-init!", driver_name);
dbg("!! Unit type %s failed to pre-init! %s", driver_name, error_get_message(rv));
clean_failed_unit(pUnit);
free(le);
return NULL;
@@ -219,9 +220,9 @@ bool ureg_load_units(PayloadParser *pp)
dbg("Adding unit \"%s\" of type %s", pUnit->name, pUnit->driver->name);
suc = pUnit->driver->init(pUnit); // finalize the load and init the unit
if (pUnit->status == E_LOADING) {
pUnit->status = suc ? E_SUCCESS : E_BAD_CONFIG;
pUnit->status = pUnit->driver->init(pUnit); // finalize the load and init the unit
if (pUnit->status != E_SUCCESS) {
dbg("!!! error initing unit %s: %s", pUnit->name, error_get_message(pUnit->status));
}
} // end unit
}
@@ -298,7 +299,7 @@ bool ureg_instantiate_by_ini(const char *restrict driver_name, const char *restr
}
/** Load unit key-value */
bool ureg_load_unit_ini_key(const char *restrict name,
error_t ureg_load_unit_ini_key(const char *restrict name,
const char *restrict key,
const char *restrict value,
uint8_t callsign)
@@ -313,10 +314,10 @@ bool ureg_load_unit_ini_key(const char *restrict name,
li = li->next;
}
return false;
return E_NO_SUCH_UNIT;
}
/** Finalize untis init */
/** Finalize units init. Returns true if all inited OK. */
bool ureg_finalize_all_init(void)
{
dbg("Finalizing units init...");
@@ -326,18 +327,13 @@ bool ureg_finalize_all_init(void)
while (li != NULL) {
Unit *const pUnit = &li->unit;
bool s = pUnit->driver->init(pUnit);
if (!s) {
dbg("!!!! error initing unit %s", pUnit->name);
if (pUnit->status == E_LOADING) {
// assume it's a config error if not otherwise specified
pUnit->status = E_BAD_CONFIG;
}
} else {
pUnit->status = E_SUCCESS;
pUnit->status = pUnit->driver->init(pUnit);
if (pUnit->status != E_SUCCESS) {
dbg("!!! error initing unit %s: %s", pUnit->name, error_get_message(pUnit->status));
}
// try to assign unique callsigns
// FIXME this is wrong, sometimes leads to duplicate CS
if (pUnit->callsign == 0) {
pUnit->callsign = callsign++;
} else {
@@ -346,7 +342,7 @@ bool ureg_finalize_all_init(void)
}
}
suc &= s;
suc &= (pUnit->status == E_SUCCESS);
li = li->next;
}
return suc;
@@ -358,7 +354,7 @@ static void export_unit_do(UlistEntry *li, IniWriter *iw)
iw_section(iw, "%s:%s@%d", pUnit->driver->name, pUnit->name, (int)pUnit->callsign);
if (pUnit->status != E_SUCCESS) {
iw_comment(iw, "!!! %s", error_get_string(pUnit->status));
iw_comment(iw, "!!! %s", error_get_message(pUnit->status));
}
pUnit->driver->cfgWriteIni(pUnit, iw);
}
@@ -445,10 +441,8 @@ void ureg_deliver_unit_request(TF_Msg *msg)
bool confirmed = (bool) (command & 0x80);
command &= 0x7F;
if (!pp.ok) { dbg("!! pp not OK!"); }
if (callsign == 0 || !pp.ok) {
com_respond_malformed_cmd(msg->frame_id);
com_respond_error(msg->frame_id, E_MALFORMED_COMMAND);
return;
}
@@ -456,12 +450,14 @@ void ureg_deliver_unit_request(TF_Msg *msg)
while (li != NULL) {
Unit *const pUnit = &li->unit;
if (pUnit->callsign == callsign && pUnit->status == E_SUCCESS) {
bool ok = pUnit->driver->handleRequest(pUnit, msg->frame_id, command, &pp);
error_t rv = pUnit->driver->handleRequest(pUnit, msg->frame_id, command, &pp);
// send extra SUCCESS confirmation message.
// error is expected to have already been reported.
if (ok && confirmed) {
com_respond_ok(msg->frame_id);
if (rv == E_SUCCESS) {
if (confirmed) com_respond_ok(msg->frame_id);
} else {
com_respond_error(msg->frame_id, rv);
}
return;
}
@@ -469,7 +465,7 @@ void ureg_deliver_unit_request(TF_Msg *msg)
}
// Not found
com_respond_snprintf(msg->frame_id, MSG_ERROR, "NO UNIT @ %"PRIu8, callsign);
com_respond_error(msg->frame_id, E_NO_SUCH_UNIT);
}
/** Send a response for a unit-list request */
@@ -492,7 +488,11 @@ void ureg_report_active_units(TF_ID frame_id)
bool suc = true;
uint8_t *buff = malloc_ck(msglen, &suc);
if (!suc) { com_respond_str(MSG_ERROR, frame_id, "OUT OF MEMORY"); return; }
if (!suc) {
com_respond_error(frame_id, E_OUT_OF_MEM);
return;
}
{
PayloadBuilder pb = pb_start(buff, msglen, NULL);
pb_u8(&pb, (uint8_t) count); // assume we don't have more than 255 units
+1 -1
View File
@@ -99,7 +99,7 @@ bool ureg_instantiate_by_ini(const char *restrict driver_name, const char *restr
* @param callsign - callsign (is part of the section string)
* @return success
*/
bool ureg_load_unit_ini_key(const char *restrict name,
error_t ureg_load_unit_ini_key(const char *restrict name,
const char *restrict key,
const char *restrict value,
uint8_t callsign);