Skip to content

Commit d76dc05

Browse files
Evgeni Raikhelclaude
andcommitted
Add typed composite-option cast helpers, document future MinZ versioning
rs_options.hpp: get_composite_option_as<T>()/get_composite_option_range_as<TRange>()/ set_composite_option_from<T>() wrap the existing raw-bytes methods with a size check and, for structs carrying a `version` field (detected via SFINAE, no opt-in needed), a check that it isn't the zero/"never populated" sentinel. Header-only, so this adds no new exported symbols - the composite-option ABI waist is unchanged. rs_hkr_minz_control.h: documents the fallback pattern for a future MinZ version that outgrows the wire envelope's reserved params[5..7] room (peek the fixed-offset version byte, dispatch into a tagged union) - illustrative only, no rs2_minz_control_v2 exists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 022afef commit d76dc05

2 files changed

Lines changed: 152 additions & 0 deletions

File tree

include/librealsense2/h/rs_hkr_minz_control.h

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,56 @@ typedef struct rs2_minz_control_range
8686
rs2_minz_control def;
8787
} rs2_minz_control_range;
8888

89+
/* ---------------------------------------------------------------------------------------------
90+
* A NOTE ON FUTURE VERSIONING (documentation only - nothing below this point is compiled, and no
91+
* rs2_minz_control_v2 exists today; this is a provision, not a feature).
92+
* ---------------------------------------------------------------------------------------------
93+
* rs2_minz_control::version exists so a future firmware revision can change this control's shape
94+
* without breaking older hosts. The wire envelope already reserves params[5..7] for exactly that
95+
* - the expected growth path is a new version populating a previously-reserved slot and bumping
96+
* param_count. sizeof(rs2_minz_control) does not change in that case, and nothing below is
97+
* needed: get_composite_option_as<rs2_minz_control>() keeps working unmodified, and it is
98+
* param_count (not the C++ type) that tells the caller how many fields are meaningful.
99+
*
100+
* If a future version ever outgrows the envelope's reserved room - a genuine breaking wire
101+
* change - sizeof() DOES change, and no C++ trick lets `auto` deduce "whichever struct matches
102+
* whatever version comes back at runtime": `auto` only ever deduces the one type a function is
103+
* declared to return. The pattern to reach for at that point is: peek the version field (always
104+
* fixed at byte 0 - this struct's one permanent invariant) BEFORE committing to a struct type,
105+
* then dispatch into a small tagged union that IS the one fixed type the caller's `auto` binds
106+
* to:
107+
*
108+
* typedef struct rs2_minz_control_v2 { ... } rs2_minz_control_v2; // whatever v2 becomes
109+
*
110+
* struct rs2_minz_control_any
111+
* {
112+
* uint8_t version; // which member of `as` is populated
113+
* union { rs2_minz_control v1; rs2_minz_control_v2 v2; } as;
114+
* };
115+
*
116+
* rs2_minz_control_any get_minz_control_versioned( const rs2::options & sensor, rs2_composite_option_id id )
117+
* {
118+
* std::vector<uint8_t> raw = sensor.get_composite_option( id ); // no cast yet
119+
* rs2_minz_control_any result{};
120+
* result.version = raw[0]; // fixed offset, by contract
121+
* switch( result.version )
122+
* {
123+
* case 1: memcpy( &result.as.v1, raw.data(), sizeof( rs2_minz_control ) ); break;
124+
* case 2: memcpy( &result.as.v2, raw.data(), sizeof( rs2_minz_control_v2 ) ); break;
125+
* default: throw std::runtime_error( "unrecognized MinZ control wire version" );
126+
* }
127+
* return result;
128+
* }
129+
*
130+
* auto cfg = get_minz_control_versioned( sensor, id ); // auto -> rs2_minz_control_any, fixed
131+
* if( cfg.version == 1 ) { use cfg.as.v1 ... }
132+
*
133+
* This tagged-union shape works unmodified in C99 too - no templates involved, just a struct
134+
* with a tag and a union. It is a SEPARATE versioning axis from rs2_minz_control_range::version
135+
* above, which versions the {min,max,step,def} WRAPPER shape, not this struct's own payload
136+
* shape - the two can change independently.
137+
* --------------------------------------------------------------------------------------------- */
138+
89139
#ifdef __cplusplus
90140
}
91141
#endif

include/librealsense2/hpp/rs_options.hpp

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212
#include <vector>
1313
#include <cstdint>
1414
#include <cstddef>
15+
#include <cstring>
16+
#include <string>
17+
#include <stdexcept>
18+
#include <type_traits>
19+
#include <utility>
1520

1621

1722
namespace rs2
@@ -372,6 +377,53 @@ namespace rs2
372377
return unwrap_raw_data_buffer( buffer, e );
373378
}
374379

380+
/**
381+
* typed counterpart to get_composite_option() - casts the raw payload directly into T
382+
* instead of handing back bytes for the caller to memcpy themselves. Validates that the
383+
* device returned exactly sizeof(T) bytes, and - only for structs that carry a `version`
384+
* field, e.g. rs2_minz_control - that it isn't the "never populated" sentinel (0).
385+
* \param[in] id composite option id to read
386+
* \return T, populated from the option's current raw payload
387+
*/
388+
template< typename T >
389+
T get_composite_option_as( rs2_composite_option_id id ) const
390+
{
391+
T value{};
392+
cast_composite_payload( get_composite_option( id ), value );
393+
return value;
394+
}
395+
396+
/**
397+
* typed counterpart to get_composite_option_range() - casts the raw {min,max,step,def}
398+
* payload directly into TRange (e.g. rs2_minz_control_range) instead of handing back
399+
* bytes. Same size/version validation as get_composite_option_as().
400+
* \param[in] id composite option id to read
401+
* \return TRange, populated from the option's raw range payload
402+
*/
403+
template< typename TRange >
404+
TRange get_composite_option_range_as( rs2_composite_option_id id ) const
405+
{
406+
TRange range{};
407+
cast_composite_payload( get_composite_option_range( id ), range );
408+
return range;
409+
}
410+
411+
/**
412+
* typed counterpart to set_composite_option() - sends value itself instead of a raw
413+
* pointer+size pair. For structs that carry a `version` field, rejects a value that is
414+
* still 0 (default-initialized) instead of silently sending an unpopulated wire header -
415+
* the most likely real bug being caught here is a get-modify-set that only touched one
416+
* field and left the rest, header included, zero-initialized.
417+
* \param[in] id composite option id to write
418+
* \param[in] value the caller's struct matching the option's documented wire layout
419+
*/
420+
template< typename T >
421+
void set_composite_option_from( rs2_composite_option_id id, const T & value ) const
422+
{
423+
check_version_populated( value );
424+
set_composite_option( id, &value, sizeof( T ) );
425+
}
426+
375427
/**
376428
* check if particular composite option is supported (and currently enabled)
377429
* \param[in] id composite option id to be checked
@@ -476,6 +528,56 @@ namespace rs2
476528
return result;
477529
}
478530

531+
// ---- typed composite-option cast helpers (get_composite_option_as() and friends) -----
532+
//
533+
// Detects whether T has a `.version` member WITHOUT requiring T to opt in explicitly -
534+
// this alone covers every composite-option struct in the SDK today: both range wrappers
535+
// (rs2_temporal_filter_dpp_range::version, rs2_minz_control_range::version) and any value
536+
// struct that embeds its own wire header (rs2_minz_control::version). A struct with no
537+
// such field (rs2_temporal_filter_dpp_config) falls through to the no-op overload below -
538+
// there is nothing to check for it.
539+
template< typename U >
540+
class has_version_member
541+
{
542+
template< typename V > static auto test( int ) -> decltype( std::declval< V >().version, std::true_type{} );
543+
template< typename > static std::false_type test( ... );
544+
public:
545+
static const bool value = decltype( test< U >( 0 ) )::value;
546+
};
547+
548+
template< typename T >
549+
static typename std::enable_if< ! has_version_member< T >::value >::type
550+
check_version_populated( const T & )
551+
{
552+
// T has no version field (e.g. rs2_temporal_filter_dpp_config) - nothing to check.
553+
}
554+
555+
template< typename T >
556+
static typename std::enable_if< has_version_member< T >::value >::type
557+
check_version_populated( const T & value )
558+
{
559+
// Convention across every struct that carries this field (see rs_composite_option.h
560+
// and the per-feature headers it points to): valid versions start at 1. A 0 means
561+
// "never populated" - either get_composite_option_as() got back fewer meaningful
562+
// bytes than expected, or set_composite_option_from() is about to send a
563+
// default-initialized header the device never asked for.
564+
if( value.version == 0 )
565+
throw std::runtime_error( "composite option struct has an unpopulated version field (0) "
566+
"- wrong struct for this option id, or a zero-initialized "
567+
"header about to be sent?" );
568+
}
569+
570+
template< typename T >
571+
static void cast_composite_payload( const std::vector< uint8_t > & raw, T & out )
572+
{
573+
if( raw.size() != sizeof( T ) )
574+
throw std::runtime_error( "composite option payload size (" + std::to_string( raw.size() )
575+
+ ") does not match sizeof(T) (" + std::to_string( sizeof( T ) )
576+
+ ") - wrong struct for this option id?" );
577+
std::memcpy( &out, raw.data(), sizeof( T ) );
578+
check_version_populated( out );
579+
}
580+
479581
rs2_options* _options;
480582
};
481583

0 commit comments

Comments
 (0)