mirror of
https://github.com/DarkFlippers/unleashed-firmware.git
synced 2026-09-16 12:32:36 +00:00
Mykhailo ShevchukandClaude Opus 5
5629fc5e17
* SubGhz: share the remaining duplicated serialize handlers Follow-up toaf2370573. Two families of decoder serialize handlers were still copied per protocol: the one writing generic.data_2 as "Data" (beninca_arc, jarolift, kinggates_stylo_4k) and the one writing TE (holtek_th12x, legrand, smc5326). Both now live in common.c and are pointed at straight from the vtable, so the per-protocol functions and their declarations are gone. The data_2 block also existed three more times in the encoder create_data paths of those same files; they share the helper too. princeton is that TE handler plus a Guard_time write, so it calls the helper and adds its own field. honeywell had generic and decoder in the reverse order, which was the only reasonaf2370573could not fold six of its handlers - swapping the two members folds them. The helpers log through generic.protocol_name instead of each file's TAG, so they match the vtable slot signature and need no per-protocol wrapper. Two diagnostics-only consequences: converted paths log under the protocol's user-facing name rather than TAG, and the encoder create_data failure message merges with the serialize one. Nothing in the tree filters on log tags. SUBGHZ_ASSERT_*_LAYOUT pins the punned prefixes at compile time. Pinning the tail offset alone would not have caught honeywell's swap: decoder and generic are both multiples of 8, so te lands at the same offset either way. Verified by restoring the old order and watching the build fail. .text 696,740 -> 695,828 (-912 B), .rodata unchanged, api_symbols.csv untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * SubGhz: share the protocol alloc bodies 48 decoder and 48 encoder alloc functions differed only in the struct type, the protocol descriptor and, for encoders, the repeat and upload-buffer constants. They now call subghz_protocol_{decoder,encoder}_common_alloc and keep a thunk each - the vtable slot passes only SubGhzEnvironment*, so the per-protocol constants have to come from somewhere. Keeping the thunk rather than making alloc data-driven is deliberate. Hanging instance_size/repeat/size_upload off SubGhzProtocol and letting alloc be NULL would remove all 96 thunks, but it moves the same constants into .rodata at about 12 B per protocol across the whole registry, adds a dispatch branch, and changes a struct that is FAP ABI (raw.h is an exported SDK header and includes base.h). Net saving is roughly zero. SubGhzProtocolEncoderCommon stays {base, encoder} so protocols that only use free/stop/yield are not forced to carry a generic; the alloc helper puns through a nested SubGhzProtocolEncoderCommonGeneric instead, mirroring how SubGhzProtocolDecoderCommonTe extends the decoder type. SUBGHZ_ASSERT_*_LAYOUT now covers all 114 structs any shared helper puns through, not just the ones converted here - 14 of those were already being punned by free/stop/yield/reset/serialize with no guard at all. The asserts cost no flash. 23 allocs keep their own body for doing real extra work (keystore setup, rainbow-table names, extra buffers). Most could call the helper and then add their own lines the way princeton does for serialize, worth roughly 440 B; left for a follow-up so this commit stays mechanical. .text 695,828 -> 693,396 (-2,432 B), .rodata unchanged, api_symbols.csv untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * SubGhz: share the deserialize bodies that only differ by a callback Two more shared helpers in common.c: - subghz_protocol_encoder_common_deserialize(ctx, ff, min_count_bit, get_upload) covers 11 encoders whose bodies were identical apart from the bit count and their get_upload - subghz_protocol_decoder_common_deserialize_te(ctx, ff, min_count_bit) covers the three decoders that read TE back, mirroring the serialize_te helper The per-protocol get_upload functions now take void* instead of being cast to the callback type. Calling through an incompatible function pointer type is formally undefined even though it is harmless on AAPCS, and enabling LTO is on the roadmap for this same effort - not a combination worth keeping. Deliberately not converted: The 35 decoders whose deserialize is just a tail call to subghz_block_generic_deserialize_check_count_bit compile to 8 bytes each. A decoder thunk is 6 bytes, so folding them would gain about 70 B - real, but a poor return for touching 35 files and adding a second hop to a call that is already a tail call into shared code. The six encoders that call a per-protocol check_remote_controller before get_upload would need a fifth argument, which spills to the stack on AAPCS and costs more per thunk than it saves. They can instead be folded into the helper above by giving their get_upload a bool return and moving the check inside it, which also reaches allstar_firefly/elplast and, once the shared stop resets encoder.front, came_twee/marantec/power_smart/hormann/magellan - about 390 B across 13 protocols with no new helper. Left as a follow-up because it changes those functions rather than just moving them. .text 693,396 -> 692,900 (-496 B), .rodata unchanged, api_symbols.csv untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Storage: share the command-dispatch bodies Twelve public storage calls had the same S_API macro body and differed only in the StorageCommand they sent. Four static helpers now carry it: storage_file_command_bool file_truncate, file_sync, file_eof, dir_rewind storage_file_command_uint64 file_tell, file_size storage_sd_command sd_format, sd_unmount, sd_mount, sd_status storage_path_command common_remove, common_mkdir Split by return macro on purpose - S_RETURN_BOOL, S_RETURN_UINT64 and S_RETURN_ERROR read different members of the SAReturn union, so one helper for all twelve would read the wrong member. Each pairing was checked against what storage_processing.c actually writes. The public symbols stay as one-line thunks: all twelve are exported in api_symbols.csv, so the bodies could move but the symbols could not. file_close and dir_close share the shape but do work between the epilogue and the return, so they keep their own bodies. .text 692,900 -> 692,296 (-604 B), .rodata unchanged, api_symbols.csv untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * SubGhz: fold the allocs and deserializes the earlier reviews left behind Follow-up to84aca75eband5f4f4cc2b, both of which deliberately stopped at the mechanical cases and listed what they skipped. 15 more allocs now call the shared helpers and keep only their own extra work - keystore lookups, rainbow-table names, extra buffers: alutech_at_4n, beninca_arc, bin_raw, dickert_mahs, faac_slh, jarolift, keeloq, kinggates_stylo_4k, nice_flor_s, telcoma_edge. 8 more encoders join subghz_protocol_encoder_common_deserialize. Their get_upload had no failure path and returned void, so it becomes bool/void* and returns true; five of them also move their check_remote_controller call from deserialize into the top of get_upload, which needs a forward declaration since get_upload is defined above it. min_count_bit widens from uint8_t to uint16_t in both deserialize helpers. subghz_block_generic_deserialize_check_count_bit compares it against a uint16_t, so the narrower parameter would have silently truncated any protocol above 255 bits. None exists today; the tree's largest is 128. The get_upload doc says "true Always; this encoder has no failure path" rather than "on success". These eight have no bound check, unlike the eleven converted in5f4f4cc2b, and claiming success would imply a validation none of them do. They cannot overrun today - deserialize_check_count_bit pins data_count_bit by equality before get_upload runs - but keyfinder has only 4 spare entries in a 60-entry buffer, so adding the guards is worth its own commit. Not done here, deliberately: the five moved check_remote_controller calls write only serial/btn/cnt on the encoder instance, which nothing reads, so they could be deleted outright rather than moved. Deleting a store is a behaviour change and belongs with the encoder.front reset in its own commit. .text 692,296 -> 691,848 (-448 B), .rodata unchanged, api_symbols.csv untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * upd changelog Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>