* remove unnecessary nullptr check, these are always called from an
initialized AP_DroneCAN so if it's nullptr something has gone
horrifically wrong
* pass in driver index instead of repeatedly calling function to get it
* simplify error handling; knowing exactly which allocation failed is not
super helpful and one failing likely means subsequent ones will too,
as it can only fail due to being out of memory
if a user has set CAN_D1_UC_ESC_RV which is the mask of ESCs that are
reversible we were sending -8191 when disarmed, which is full reverse
throttle. This is the correct output when armed as it is treated as
full reverse at "PWM" 1000 and stopped at 1500, but when disarmed we
should always send zero or the user may find all ESCs spin up at full
reverse when disarmed if the ESC supports reverse throttle (which is
rare in DroneCAN ESCs)
The timeout specified for auxiliary driver frames was passed to the
driver where a deadline was expected. The transmission was then started
after its "deadline", thereby causing it to be canceled and the data
lost if the frame could not be sent immediately.
Fix by converting the timeout to a deadline before passing to the
driver. The conversion is done in the Canard interface code as it
already does other conversions from timeouts to deadlines.
Avoids confusing the user and removes weirdness with multiple servers
sharing the same storage. Does leak the registration for the old ID but
in the unlikely event the table fills up the user can simply reset the
database.
We keep the check for an existing registration to avoid dirtying the
storage every boot unnecessarily. We also factor out the deletion of an
existing registration (which is very unlikely but technically possible)
to save some flash.
While technically legal, it's unlikely to have been tested and an
allocatee might do silly things. Also makes the logic a bit more clear
and improves the failure message.
Rename NodeData to NodeRecord to provide a more specific name and match
the record term used in the comments. Also nominally allows stuff to be
associated with the nodes that's not the record, as expanding the record
is hard.
Additionally, rename the `hwid` field to `uid` as that's what's used in
the rest of the code.
All the higher level database operations need to be locked for the whole
duration of the operation, so nobody should be using the lower-level
tasks or raw read/write functions. We can also remove the locks from
them.
The database can now safely be used by multiple servers.
Must be locked for the whole operation due to the find free/add
read-modify-write.
Preserves the previous behavior of sending back an ID of 0 in case of
allocation failure, for better or worse.
There is (currently) only one storage area that is used by all servers,
so it needs to be managed by its own class shared among them.
The occupied mask is also moved as it reflects the storage contents and
so can't be stored by each server.
Put documentation with each bitmask and use the object directly. Node ID
range checks can be removed as the bitmask itself checks and we don't
expect to trip them.
Substantially cleans up the code.
The StorageManager read_block/write_block methods only return failure if
an out of bounds access is performed. Assert statically that this does
not happen.
Also remove the now-impossible failed to add node state.
Thomas Watson
Andrew Tridgell
Peter Barker
bugobliterator
Bob Long
olliw42
Iampete1
Pradeep CK
Patrick José Pereira