Hello builat

A couple of things have been mentioned already. Apart from that there are 2 things that I find pretty bad about your code:

a) _device_read() is misnamed

sub _device_read{
   my $d = shift;  # reduced line to relevant parts

   ...

   # parse device config
   $d->{regs}=from_json( $d->{regs} );
[download]

You are modifying the provided hash reference but the function name implies no such thing. A more suitable description would be something like '_convert_registers_and_device_read' if this is intentional or assign the result of from_json($d->{regs}) to a temporary variable if it is not. As a third option you could extract the JSON-conversion into a different function.

b) error handling

sub _device_read{
  ...
  $r->{$reg} = _prittyfy_byte(...);
  ...
};

sub _prettify_byte{
    return (...) if( $_[0] );
    return( $c{_err_} );
};
[download]

If prettify fails, then it returns a hash key from a global variable? Why even bother to return something if it's a static string? Why not simply return undef? (If it's an error message, then you can access $c{_err_} later on anyway?)

additional comment 1: Why do you convert a byte into a float in the first place?

additional comment 2: How does this work? ('_prettify_byte([1, 2])' results in 0.00)

General comments

Technically there's nothing wrong with optimizing your code to use as few variables as possible. From a maintenance / debug POV I think that is bad. If I can split a computation into separate steps then usually I put the value into a suitably named variable. My loop could end up like this:

my %registers = %{$d->{regs}->{r}};
foreach my $reg (keys %registers){
  my $reg_id  = hex $reg;
  my $reg_num = $registers{$reg};

  # read_input_registers returns an array reference
  my $tmp = $mbc->read_input_registers($reg_id, $reg_num);

  $r->{$reg} = _prettify_byte($tmp);
};
[download]

or maybe like this:

my %registers = %{$d->{regs}->{r}};
foreach my $reg (keys %registers){
  # read_input_registers returns an array reference
  my $tmp = $mbc->read_input_registers(hex $reg, $registers{$reg});

  $r->{$reg} = _prettify_byte($tmp);
};
[download]

...but not as a single statement that spans multiple lines and calls 2 different functions.

Why? I'm optimizing for debugging / maintenance and not for performance. If performance turns out to be an issue, then I can still optimize for it later on. But first I need to get the code correct. Having variables I can inspect and validate is a nice thing. Being able to tell at a glance if a statement looks reasonable is another.

Apart from that: I dislike your coding convention regarding $d and $r. I think it's totally fine to use a single letter variable inside a small loop (let's say 4 lines of code). For everything longer than that I'd like to have a descriptive name. Even if it's just 'input' and 'result'. If it works for you, then fine. I'm just saying I don't like it.