Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow scripting to override notify displays #15857

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions libraries/AP_Notify/AP_Notify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,16 @@ void AP_Notify::handle_rgb(uint8_t r, uint8_t g, uint8_t b, uint8_t rate_hz)
}
}

// handle display override from scripting
void AP_Notify::handle_scr_disp(uint8_t r, const char *str)
{
for (uint8_t i = 0; i < _num_devices; i++) {
if (_devices[i] != nullptr) {
_devices[i]->scr_disp_overide(r, str);
}
}
}

// handle a PLAY_TUNE message
void AP_Notify::handle_play_tune(const mavlink_message_t &msg)
{
Expand Down
3 changes: 3 additions & 0 deletions libraries/AP_Notify/AP_Notify.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ class AP_Notify
// handle a PLAY_TUNE message
static void handle_play_tune(const mavlink_message_t &msg);

// handle display override from scripting
static void handle_scr_disp(uint8_t r, const char *str);

// play a tune string
static void play_tune(const char *tune);

Expand Down
26 changes: 26 additions & 0 deletions libraries/AP_Notify/Display.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,17 @@ void Display::update()
}
timer = 0;

// If we have received an override from scripting recently allow update
if (AP_HAL::millis() - _last_scr_override <= _script_timeout_ms) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this block the display for the first 5 seconds after boot? You probably need a boolean for "we have overrides" and then once outside the time window set the boolean back to false.

if (_screenpage != 3) {
_driver->clear_screen();
_screenpage = 3;
}
update_scr_screen();
_driver->hw_update();
return;
}

if (AP_Notify::flags.armed) {
if (_screenpage != 1) {
_driver->clear_screen();
Expand Down Expand Up @@ -582,3 +593,18 @@ void Display::update_text(uint8_t r)

draw_text(COLUMN(0), ROW(0), msg);
}

// Allow scripting to override text lines on the display
void Display::scr_disp_overide(uint8_t r, const char *str) {
// Prevent chars being left from previous message by clearing with spaces first
memset(_scr_msg[r], ' ', DISPLAY_MESSAGE_SIZE-1); // leave null termination
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just memset to 0?

strncpy(_scr_msg[r], str, strlen(str));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to check that str is shorter than DISPLAY_MESSAGE_SIZE-1

Comment on lines +600 to +601
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
memset(_scr_msg[r], ' ', DISPLAY_MESSAGE_SIZE-1); // leave null termination
strncpy(_scr_msg[r], str, strlen(str));
strncpy(_scr_msg[r], str, sizeof(_scr_msg[r]) - 1);
       The  strncpy()  function is similar, except that at most n bytes of src are copied.  Warning: If there is no null
       byte among the first n bytes of src, the string placed in dest will not be null-terminated.

       If the length of src is less than n, strncpy() writes additional null bytes to dest to ensure that a total  of  n
       bytes are written.


_last_scr_override = AP_HAL::millis();
}

void Display::update_scr_screen() {
for (uint8_t i = 0; i < 6; i++) {
draw_text(COLUMN(0), ROW(i), _scr_msg[i]);
}
}
11 changes: 11 additions & 0 deletions libraries/AP_Notify/Display.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ class Display: public NotifyDevice {
bool init(void) override;
void update() override;

// Allows scripting to override the display message
void scr_disp_overide(uint8_t r, const char *str) override;

private:
void draw_char(uint16_t x, uint16_t y, const char c);
void draw_text(uint16_t x, uint16_t y, const char *c);
Expand All @@ -36,6 +39,14 @@ class Display: public NotifyDevice {
uint8_t _movedelay; // ticker delay before shifting after new message displayed
uint8_t _screenpage;

void update_scr_screen(void);

// stop scripting override if we have not recieved anything for 5 sec
static const uint16_t _script_timeout_ms = 5000;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be a #define, or just hard code it with a comment if its only used in one spot

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static const is generally better then #define


uint32_t _last_scr_override;
char _scr_msg[6][DISPLAY_MESSAGE_SIZE] = {};

// stop showing text in display after this many millis:
const uint16_t _send_text_valid_millis = 20000;
};
Expand Down
3 changes: 3 additions & 0 deletions libraries/AP_Notify/NotifyDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ class NotifyDevice {
// give RGB and flash rate, used with scripting
virtual void rgb_control(uint8_t r, uint8_t g, uint8_t b, uint8_t rate_hz) {}

// Allows scripting to override the display message
virtual void scr_disp_overide(uint8_t r, const char *str) {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
virtual void scr_disp_overide(uint8_t r, const char *str) {}
virtual void scr_disp_overide(uint8_t line, const char *str) {}

should make it a little clearer.


// this pointer is used to read the parameters relative to devices
const AP_Notify *pNotify;
};
37 changes: 37 additions & 0 deletions libraries/AP_Scripting/examples/use_notify_display.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
-- This is a simple script example that shows how to use the notify display override.
-- In this example battery voltage and percent remaining is retrieved and displayed
-- on screen to act as a handy onboard display for checking the batteries.

local function update()

local voltage = 0
local remaining = 0
local healthy = battery:healthy(0)

-- Update display
-- First argument is line number. There are up to 6 lines available. 0 based numbering is used
-- Second argument is the string to be displayed. Up to 19 characters can be displayed on each line
notify:handle_scr_disp(0," --- ArduPilot --- ")

if (healthy) then
-- Get voltage and display it on line 3
voltage = battery:voltage(0)
notify:handle_scr_disp(2, string.format(" Voltage: %.2f V", voltage))

-- Get pack remaining and display it on line 5
remaining = battery:capacity_remaining_pct(0)
notify:handle_scr_disp(4, string.format("Remaining: %.2f %%", remaining))

else
-- voltage monitor is unhealthy
notify:handle_scr_disp(3, " Battery Monitor ")
notify:handle_scr_disp(4, " Unhealthy ")
end

-- Display is updated at 2 Hz so no point in calling any faster than that
return update, 500

end

-- Wait for 5 sec before starting script
return update, 5000
1 change: 1 addition & 0 deletions libraries/AP_Scripting/generator/description/bindings.desc
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ include AP_Notify/AP_Notify.h
singleton AP_Notify alias notify
singleton AP_Notify method play_tune void string
singleton AP_Notify method handle_rgb void uint8_t 0 UINT8_MAX uint8_t 0 UINT8_MAX uint8_t 0 UINT8_MAX uint8_t 0 UINT8_MAX
singleton AP_Notify method handle_scr_disp void uint8_t 0 5 string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bad name for the method from a script side. Something like set_display_text() maybe?


include AP_Proximity/AP_Proximity.h

Expand Down